From 03a36e3398781dd1967721a4e9aa88d6e1c8e2f4 Mon Sep 17 00:00:00 2001 From: Brandon Palm Date: Tue, 18 Aug 2026 16:37:46 -0500 Subject: [PATCH] CM-1113: Replace unsafe.Pointer casts with Kubernetes conversion functions - Replaces all 8 raw unsafe.Pointer casts between corev1.* and core.* types with Kubernetes auto-generated conversion functions from k8s.io/kubernetes/pkg/apis/core/v1 - Removes "unsafe" import from 3 files - Adds ToCoreTolerations and ToV1Tolerations helpers in pkg/controller/common/validation.go Includes unit tests for: - Validation functions (labels, annotations, nodeSelector, tolerations, resources, affinity) - Override helpers (args, env, pod labels) Co-authored-by: Cursor --- .../certmanager/deployment_helper.go | 17 +- .../certmanager/deployment_helper_test.go | 294 +++++++++++++++++- .../deployment_overrides_validation.go | 7 +- pkg/controller/common/validation.go | 40 ++- pkg/controller/common/validation_test.go | 237 ++++++++++++++ pkg/tlsprofile/tlsprofile_test.go | 14 + 6 files changed, 579 insertions(+), 30 deletions(-) create mode 100644 pkg/controller/common/validation_test.go diff --git a/pkg/controller/certmanager/deployment_helper.go b/pkg/controller/certmanager/deployment_helper.go index 8b34e0abd..b79acb1b9 100644 --- a/pkg/controller/certmanager/deployment_helper.go +++ b/pkg/controller/certmanager/deployment_helper.go @@ -3,14 +3,13 @@ package certmanager import ( "fmt" "sort" - "unsafe" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/labels" - "k8s.io/kubernetes/pkg/apis/core" "k8s.io/kubernetes/pkg/util/tolerations" "github.com/openshift/cert-manager-operator/api/operator/v1alpha1" + "github.com/openshift/cert-manager-operator/pkg/controller/common" certmanagerinformer "github.com/openshift/cert-manager-operator/pkg/operator/informers/externalversions/operator/v1alpha1" ) @@ -75,15 +74,11 @@ func mergePodScheduling(sourceScheduling v1alpha1.CertManagerScheduling, overrid // Merge the source and override NodeSelector. mergedNodeSelector := labels.Merge(sourceScheduling.NodeSelector, overrideScheduling.NodeSelector) - // Convert corev1.Tolerations to core.Tolerations. - sourceTolerations := *(*[]core.Toleration)(unsafe.Pointer(&sourceScheduling.Tolerations)) - overridingTolerations := *(*[]core.Toleration)(unsafe.Pointer(&overrideScheduling.Tolerations)) - - // Merge the source and override Tolerations. - mergedCoreTolerations := tolerations.MergeTolerations(sourceTolerations, overridingTolerations) - - // Convert core.Tolerations to corev1.Tolerations. - mergedCorev1Tolerations := *(*[]corev1.Toleration)(unsafe.Pointer(&mergedCoreTolerations)) + mergedCoreTolerations := tolerations.MergeTolerations( + common.ToCoreTolerations(sourceScheduling.Tolerations), + common.ToCoreTolerations(overrideScheduling.Tolerations), + ) + mergedCorev1Tolerations := common.ToV1Tolerations(mergedCoreTolerations) return v1alpha1.CertManagerScheduling{ NodeSelector: mergedNodeSelector, diff --git a/pkg/controller/certmanager/deployment_helper_test.go b/pkg/controller/certmanager/deployment_helper_test.go index b48041d0a..e260bd378 100644 --- a/pkg/controller/certmanager/deployment_helper_test.go +++ b/pkg/controller/certmanager/deployment_helper_test.go @@ -425,7 +425,7 @@ func TestGetOverrideResourcesFor(t *testing.T) { withFakeCertManagerForTest(t, ctx, fakeClient, certManagerChan, &tc.certManagerObj) actualOverrideResources, err := getOverrideResourcesFor(certManagerInformers, tc.deploymentName) - assert.NoError(t, err) + require.NoError(t, err) require.Equal(t, tc.expectedOverrideResources, actualOverrideResources) }) } @@ -872,12 +872,300 @@ func TestGetOverrideSchedulingFor(t *testing.T) { withFakeCertManagerForTest(t, ctx, fakeClient, certManagerChan, &tc.certManagerObj) actualOverrideScheduling, err := getOverrideSchedulingFor(certManagerInformers, tc.deploymentName) - assert.NoError(t, err) + require.NoError(t, err) require.Equal(t, tc.expectedOverrideScheduling, actualOverrideScheduling) }) } } +func TestGetOverrideArgsFor(t *testing.T) { + tests := []struct { + name string + certManagerObj v1alpha1.CertManager + deploymentName string + expectedArgs []string + expectError bool + errContains string + }{ + { + name: "get override args for controller", + certManagerObj: v1alpha1.CertManager{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: v1alpha1.CertManagerSpec{ + ControllerConfig: &v1alpha1.DeploymentConfig{ + OverrideArgs: []string{"--v=4", "--feature-gates=ExperimentalGatewayAPISupport=true"}, + }, + }, + }, + deploymentName: certmanagerControllerDeployment, + expectedArgs: []string{"--v=4", "--feature-gates=ExperimentalGatewayAPISupport=true"}, + }, + { + name: "get override args for webhook", + certManagerObj: v1alpha1.CertManager{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: v1alpha1.CertManagerSpec{ + WebhookConfig: &v1alpha1.DeploymentConfig{ + OverrideArgs: []string{"--secure-port=10251"}, + }, + }, + }, + deploymentName: certmanagerWebhookDeployment, + expectedArgs: []string{"--secure-port=10251"}, + }, + { + name: "get override args for cainjector", + certManagerObj: v1alpha1.CertManager{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: v1alpha1.CertManagerSpec{ + CAInjectorConfig: &v1alpha1.DeploymentConfig{ + OverrideArgs: []string{"--leader-elect=false"}, + }, + }, + }, + deploymentName: certmanagerCAinjectorDeployment, + expectedArgs: []string{"--leader-elect=false"}, + }, + { + name: "nil config returns nil args for controller", + certManagerObj: v1alpha1.CertManager{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: v1alpha1.CertManagerSpec{}, + }, + deploymentName: certmanagerControllerDeployment, + expectedArgs: nil, + }, + { + name: "unsupported deployment name returns error", + certManagerObj: v1alpha1.CertManager{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: v1alpha1.CertManagerSpec{}, + }, + deploymentName: "unknown-deployment", + expectError: true, + errContains: "unsupported deployment name", + }, + } + + ctx := t.Context() + fakeClient, certManagerInformers, certManagerChan := setupSyncedFakeCertManagerInformer(t, ctx) + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + withFakeCertManagerForTest(t, ctx, fakeClient, certManagerChan, &tc.certManagerObj) + + actualArgs, err := getOverrideArgsFor(certManagerInformers, tc.deploymentName) + if tc.expectError { + require.Error(t, err) + assert.Contains(t, err.Error(), tc.errContains) + } else { + require.NoError(t, err) + require.Equal(t, tc.expectedArgs, actualArgs) + } + }) + } +} + +func TestGetOverrideEnvFor(t *testing.T) { + tests := []struct { + name string + certManagerObj v1alpha1.CertManager + deploymentName string + expectedEnv []corev1.EnvVar + expectError bool + errContains string + }{ + { + name: "get override env for controller", + certManagerObj: v1alpha1.CertManager{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: v1alpha1.CertManagerSpec{ + ControllerConfig: &v1alpha1.DeploymentConfig{ + OverrideEnv: []corev1.EnvVar{ + {Name: "HTTP_PROXY", Value: "http://proxy:3128"}, + }, + }, + }, + }, + deploymentName: certmanagerControllerDeployment, + expectedEnv: []corev1.EnvVar{ + {Name: "HTTP_PROXY", Value: "http://proxy:3128"}, + }, + }, + { + name: "get override env for webhook", + certManagerObj: v1alpha1.CertManager{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: v1alpha1.CertManagerSpec{ + WebhookConfig: &v1alpha1.DeploymentConfig{ + OverrideEnv: []corev1.EnvVar{ + {Name: "MY_VAR", Value: "my-value"}, + }, + }, + }, + }, + deploymentName: certmanagerWebhookDeployment, + expectedEnv: []corev1.EnvVar{ + {Name: "MY_VAR", Value: "my-value"}, + }, + }, + { + name: "get override env for cainjector", + certManagerObj: v1alpha1.CertManager{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: v1alpha1.CertManagerSpec{ + CAInjectorConfig: &v1alpha1.DeploymentConfig{ + OverrideEnv: []corev1.EnvVar{ + {Name: "NO_PROXY", Value: "localhost"}, + }, + }, + }, + }, + deploymentName: certmanagerCAinjectorDeployment, + expectedEnv: []corev1.EnvVar{ + {Name: "NO_PROXY", Value: "localhost"}, + }, + }, + { + name: "nil config returns nil env", + certManagerObj: v1alpha1.CertManager{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: v1alpha1.CertManagerSpec{}, + }, + deploymentName: certmanagerControllerDeployment, + expectedEnv: nil, + }, + { + name: "unsupported deployment name returns error", + certManagerObj: v1alpha1.CertManager{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: v1alpha1.CertManagerSpec{}, + }, + deploymentName: "unknown-deployment", + expectError: true, + errContains: "unsupported deployment name", + }, + } + + ctx := t.Context() + fakeClient, certManagerInformers, certManagerChan := setupSyncedFakeCertManagerInformer(t, ctx) + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + withFakeCertManagerForTest(t, ctx, fakeClient, certManagerChan, &tc.certManagerObj) + + actualEnv, err := getOverrideEnvFor(certManagerInformers, tc.deploymentName) + if tc.expectError { + require.Error(t, err) + assert.Contains(t, err.Error(), tc.errContains) + } else { + require.NoError(t, err) + require.Equal(t, tc.expectedEnv, actualEnv) + } + }) + } +} + +func TestGetOverridePodLabelsFor(t *testing.T) { + tests := []struct { + name string + certManagerObj v1alpha1.CertManager + deploymentName string + expectedLabels map[string]string + expectError bool + errContains string + }{ + { + name: "get override labels for controller", + certManagerObj: v1alpha1.CertManager{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: v1alpha1.CertManagerSpec{ + ControllerConfig: &v1alpha1.DeploymentConfig{ + OverrideLabels: map[string]string{ + "custom-label": "custom-value", + }, + }, + }, + }, + deploymentName: certmanagerControllerDeployment, + expectedLabels: map[string]string{ + "custom-label": "custom-value", + }, + }, + { + name: "get override labels for webhook", + certManagerObj: v1alpha1.CertManager{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: v1alpha1.CertManagerSpec{ + WebhookConfig: &v1alpha1.DeploymentConfig{ + OverrideLabels: map[string]string{ + "env": "production", + }, + }, + }, + }, + deploymentName: certmanagerWebhookDeployment, + expectedLabels: map[string]string{ + "env": "production", + }, + }, + { + name: "get override labels for cainjector", + certManagerObj: v1alpha1.CertManager{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: v1alpha1.CertManagerSpec{ + CAInjectorConfig: &v1alpha1.DeploymentConfig{ + OverrideLabels: map[string]string{ + "team": "security", + }, + }, + }, + }, + deploymentName: certmanagerCAinjectorDeployment, + expectedLabels: map[string]string{ + "team": "security", + }, + }, + { + name: "nil config returns nil labels", + certManagerObj: v1alpha1.CertManager{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: v1alpha1.CertManagerSpec{}, + }, + deploymentName: certmanagerControllerDeployment, + expectedLabels: nil, + }, + { + name: "unsupported deployment name returns error", + certManagerObj: v1alpha1.CertManager{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: v1alpha1.CertManagerSpec{}, + }, + deploymentName: "unknown-deployment", + expectError: true, + errContains: "unsupported deployment name", + }, + } + + ctx := t.Context() + fakeClient, certManagerInformers, certManagerChan := setupSyncedFakeCertManagerInformer(t, ctx) + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + withFakeCertManagerForTest(t, ctx, fakeClient, certManagerChan, &tc.certManagerObj) + + actualLabels, err := getOverridePodLabelsFor(certManagerInformers, tc.deploymentName) + if tc.expectError { + require.Error(t, err) + assert.Contains(t, err.Error(), tc.errContains) + } else { + require.NoError(t, err) + require.Equal(t, tc.expectedLabels, actualLabels) + } + }) + } +} + func TestGetOverrideReplicasFor(t *testing.T) { tests := []struct { name string @@ -999,7 +1287,7 @@ func TestGetOverrideReplicasFor(t *testing.T) { withFakeCertManagerForTest(t, ctx, fakeClient, certManagerChan, &tc.certManagerObj) actualOverrideReplicas, err := getOverrideReplicasFor(certManagerInformers, tc.deploymentName) - assert.NoError(t, err) + require.NoError(t, err) if tc.expectedOverrideReplicas == nil { assert.Nil(t, actualOverrideReplicas) } else { diff --git a/pkg/controller/certmanager/deployment_overrides_validation.go b/pkg/controller/certmanager/deployment_overrides_validation.go index 34d4c342a..92b3d60fd 100644 --- a/pkg/controller/certmanager/deployment_overrides_validation.go +++ b/pkg/controller/certmanager/deployment_overrides_validation.go @@ -3,14 +3,12 @@ package certmanager import ( "fmt" "strconv" - "unsafe" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" metav1validation "k8s.io/apimachinery/pkg/apis/meta/v1/validation" utilerrors "k8s.io/apimachinery/pkg/util/errors" "k8s.io/apimachinery/pkg/util/validation/field" - "k8s.io/kubernetes/pkg/apis/core" corevalidation "k8s.io/kubernetes/pkg/apis/core/validation" "k8s.io/utils/strings/slices" @@ -363,10 +361,7 @@ func withPodSchedulingValidateHook(certmanagerinformer certmanagerinformer.CertM func validateScheduling(scheduling v1alpha1.CertManagerScheduling, fldPath *field.Path) error { errs := metav1validation.ValidateLabels(scheduling.NodeSelector, fldPath.Child("nodeSelector")) - // Convert corev1.Tolerations to core.Tolerations. - tolerations := *(*[]core.Toleration)(unsafe.Pointer(&scheduling.Tolerations)) - - errs = append(errs, corevalidation.ValidateTolerations(tolerations, fldPath.Child("tolerations"), corevalidation.PodValidationOptions{})...) + errs = append(errs, corevalidation.ValidateTolerations(common.ToCoreTolerations(scheduling.Tolerations), fldPath.Child("tolerations"), corevalidation.PodValidationOptions{})...) return errs.ToAggregate() } diff --git a/pkg/controller/common/validation.go b/pkg/controller/common/validation.go index 421f7a6f6..1fd68cc80 100644 --- a/pkg/controller/common/validation.go +++ b/pkg/controller/common/validation.go @@ -1,13 +1,12 @@ package common import ( - "unsafe" - corev1 "k8s.io/api/core/v1" apivalidation "k8s.io/apimachinery/pkg/api/validation" metav1validation "k8s.io/apimachinery/pkg/apis/meta/v1/validation" "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/kubernetes/pkg/apis/core" + corev1conversion "k8s.io/kubernetes/pkg/apis/core/v1" corevalidation "k8s.io/kubernetes/pkg/apis/core/validation" ) @@ -20,25 +19,26 @@ func ValidateNodeSelectorConfig(nodeSelector map[string]string, fldPath *field.P // ValidateTolerationsConfig validates the Tolerations configuration using // the Kubernetes core toleration validation rules. func ValidateTolerationsConfig(tolerations []corev1.Toleration, fldPath *field.Path) error { - // convert corev1.Tolerations to core.Tolerations, required for validation. - convTolerations := *(*[]core.Toleration)(unsafe.Pointer(&tolerations)) - return corevalidation.ValidateTolerations(convTolerations, fldPath.Child("tolerations"), corevalidation.PodValidationOptions{}).ToAggregate() + return corevalidation.ValidateTolerations(ToCoreTolerations(tolerations), fldPath.Child("tolerations"), corevalidation.PodValidationOptions{}).ToAggregate() } // ValidateResourceRequirements validates the ResourceRequirements configuration // using the Kubernetes core resource requirements validation rules. func ValidateResourceRequirements(requirements corev1.ResourceRequirements, fldPath *field.Path) error { - // convert corev1.ResourceRequirements to core.ResourceRequirements, required for validation. - convRequirements := *(*core.ResourceRequirements)(unsafe.Pointer(&requirements)) + var convRequirements core.ResourceRequirements + _ = corev1conversion.Convert_v1_ResourceRequirements_To_core_ResourceRequirements(&requirements, &convRequirements, nil) return corevalidation.ValidateContainerResourceRequirements(&convRequirements, nil, fldPath.Child("resources"), corevalidation.PodValidationOptions{}).ToAggregate() } // ValidateAffinityRules validates the Affinity configuration using // the Kubernetes core affinity validation rules. func ValidateAffinityRules(affinity *corev1.Affinity, fldPath *field.Path) error { - // convert corev1.Affinity to core.Affinity, required for validation. - convAffinity := (*core.Affinity)(unsafe.Pointer(affinity)) - return validateAffinity(convAffinity, corevalidation.PodValidationOptions{}, fldPath.Child("affinity")).ToAggregate() + if affinity == nil { + return nil + } + var convAffinity core.Affinity + _ = corev1conversion.Convert_v1_Affinity_To_core_Affinity(affinity, &convAffinity, nil) + return validateAffinity(&convAffinity, corevalidation.PodValidationOptions{}, fldPath.Child("affinity")).ToAggregate() } // ValidateLabelsConfig validates label keys and values using the Kubernetes @@ -52,3 +52,23 @@ func ValidateLabelsConfig(labels map[string]string, fldPath *field.Path) error { func ValidateAnnotationsConfig(annotations map[string]string, fldPath *field.Path) error { return apivalidation.ValidateAnnotations(annotations, fldPath.Child("annotations")).ToAggregate() } + +// ToCoreTolerations converts a slice of corev1.Toleration to core.Toleration +// using Kubernetes' auto-generated conversion functions. +func ToCoreTolerations(in []corev1.Toleration) []core.Toleration { + out := make([]core.Toleration, len(in)) + for i := range in { + _ = corev1conversion.Convert_v1_Toleration_To_core_Toleration(&in[i], &out[i], nil) + } + return out +} + +// ToV1Tolerations converts a slice of core.Toleration to corev1.Toleration +// using Kubernetes' auto-generated conversion functions. +func ToV1Tolerations(in []core.Toleration) []corev1.Toleration { + out := make([]corev1.Toleration, len(in)) + for i := range in { + _ = corev1conversion.Convert_core_Toleration_To_v1_Toleration(&in[i], &out[i], nil) + } + return out +} diff --git a/pkg/controller/common/validation_test.go b/pkg/controller/common/validation_test.go new file mode 100644 index 000000000..7a767630a --- /dev/null +++ b/pkg/controller/common/validation_test.go @@ -0,0 +1,237 @@ +package common + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" + "k8s.io/apimachinery/pkg/util/validation/field" + "k8s.io/kubernetes/pkg/apis/core" + "k8s.io/utils/ptr" +) + +func TestToCoreTolerations_RoundTrip(t *testing.T) { + input := []corev1.Toleration{ + {Key: "key1", Operator: corev1.TolerationOpExists, Effect: corev1.TaintEffectNoSchedule}, + {Key: "key2", Operator: corev1.TolerationOpEqual, Value: "val2", Effect: corev1.TaintEffectNoExecute, TolerationSeconds: ptr.To(int64(300))}, + } + core := ToCoreTolerations(input) + require.Len(t, core, 2) + roundTripped := ToV1Tolerations(core) + require.Equal(t, input, roundTripped) +} + +func TestToCoreTolerations_Nil(t *testing.T) { + require.Empty(t, ToCoreTolerations(nil)) + require.Empty(t, ToV1Tolerations(nil)) +} + +func TestToCoreTolerations_Empty(t *testing.T) { + require.Empty(t, ToCoreTolerations([]corev1.Toleration{})) + require.Empty(t, ToV1Tolerations([]core.Toleration{})) +} + +func TestValidateLabelsConfig(t *testing.T) { + fldPath := field.NewPath("spec") + + t.Run("valid labels pass", func(t *testing.T) { + labels := map[string]string{ + "app": "test", + "example.com/my-component": "frontend", + } + err := ValidateLabelsConfig(labels, fldPath) + require.NoError(t, err) + }) + + t.Run("invalid label key returns error", func(t *testing.T) { + labels := map[string]string{ + "INVALID KEY WITH SPACES": "value", + } + err := ValidateLabelsConfig(labels, fldPath) + require.Error(t, err) + }) + + t.Run("empty labels pass", func(t *testing.T) { + err := ValidateLabelsConfig(map[string]string{}, fldPath) + require.NoError(t, err) + }) +} + +func TestValidateAnnotationsConfig(t *testing.T) { + fldPath := field.NewPath("spec") + + t.Run("valid annotations pass", func(t *testing.T) { + annotations := map[string]string{ + "kubectl.kubernetes.io/last-applied-configuration": "{}", + "my-annotation": "some-value", + } + err := ValidateAnnotationsConfig(annotations, fldPath) + require.NoError(t, err) + }) + + t.Run("invalid annotation key returns error", func(t *testing.T) { + annotations := map[string]string{ + "invalid key!@#$": "value", + } + err := ValidateAnnotationsConfig(annotations, fldPath) + require.Error(t, err) + }) +} + +func TestValidateNodeSelectorConfig(t *testing.T) { + fldPath := field.NewPath("spec") + + t.Run("valid nodeSelector passes", func(t *testing.T) { + nodeSelector := map[string]string{ + "kubernetes.io/os": "linux", + "node-role": "worker", + } + err := ValidateNodeSelectorConfig(nodeSelector, fldPath) + require.NoError(t, err) + }) + + t.Run("empty value key passes", func(t *testing.T) { + nodeSelector := map[string]string{ + "node.kubernetes.io/instance-type": "", + } + err := ValidateNodeSelectorConfig(nodeSelector, fldPath) + require.NoError(t, err) + }) + + t.Run("invalid key returns error", func(t *testing.T) { + nodeSelector := map[string]string{ + "BAD KEY!!!": "value", + } + err := ValidateNodeSelectorConfig(nodeSelector, fldPath) + require.Error(t, err) + }) +} + +func TestValidateTolerationsConfig(t *testing.T) { + fldPath := field.NewPath("spec") + + t.Run("valid tolerations pass", func(t *testing.T) { + tolerations := []corev1.Toleration{ + { + Key: "node.kubernetes.io/not-ready", + Operator: corev1.TolerationOpExists, + Effect: corev1.TaintEffectNoSchedule, + }, + { + Key: "dedicated", + Operator: corev1.TolerationOpEqual, + Value: "cert-manager", + Effect: corev1.TaintEffectNoSchedule, + }, + } + err := ValidateTolerationsConfig(tolerations, fldPath) + require.NoError(t, err) + }) + + t.Run("invalid operator returns error", func(t *testing.T) { + tolerations := []corev1.Toleration{ + { + Key: "key", + Operator: corev1.TolerationOperator("InvalidOp"), + Effect: corev1.TaintEffectNoSchedule, + }, + } + err := ValidateTolerationsConfig(tolerations, fldPath) + require.Error(t, err) + }) + + t.Run("empty tolerations pass", func(t *testing.T) { + err := ValidateTolerationsConfig([]corev1.Toleration{}, fldPath) + require.NoError(t, err) + }) +} + +func TestValidateResourceRequirements(t *testing.T) { + fldPath := field.NewPath("spec") + + t.Run("valid cpu and memory pass", func(t *testing.T) { + reqs := corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("100m"), + corev1.ResourceMemory: resource.MustParse("128Mi"), + }, + Limits: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("500m"), + corev1.ResourceMemory: resource.MustParse("512Mi"), + }, + } + err := ValidateResourceRequirements(reqs, fldPath) + require.NoError(t, err) + }) + + t.Run("negative cpu returns error", func(t *testing.T) { + reqs := corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("-100m"), + }, + } + err := ValidateResourceRequirements(reqs, fldPath) + require.Error(t, err) + }) + + t.Run("empty requirements pass", func(t *testing.T) { + err := ValidateResourceRequirements(corev1.ResourceRequirements{}, fldPath) + require.NoError(t, err) + }) +} + +func TestValidateAffinityRules(t *testing.T) { + fldPath := field.NewPath("spec") + + t.Run("nil affinity passes", func(t *testing.T) { + err := ValidateAffinityRules(nil, fldPath) + require.NoError(t, err) + }) + + t.Run("valid node affinity passes", func(t *testing.T) { + affinity := &corev1.Affinity{ + NodeAffinity: &corev1.NodeAffinity{ + RequiredDuringSchedulingIgnoredDuringExecution: &corev1.NodeSelector{ + NodeSelectorTerms: []corev1.NodeSelectorTerm{ + { + MatchExpressions: []corev1.NodeSelectorRequirement{ + { + Key: "kubernetes.io/os", + Operator: corev1.NodeSelectorOpIn, + Values: []string{"linux"}, + }, + }, + }, + }, + }, + }, + } + err := ValidateAffinityRules(affinity, fldPath) + require.NoError(t, err) + }) + + t.Run("invalid label selector in node affinity returns error", func(t *testing.T) { + affinity := &corev1.Affinity{ + NodeAffinity: &corev1.NodeAffinity{ + RequiredDuringSchedulingIgnoredDuringExecution: &corev1.NodeSelector{ + NodeSelectorTerms: []corev1.NodeSelectorTerm{ + { + MatchExpressions: []corev1.NodeSelectorRequirement{ + { + Key: "kubernetes.io/os", + Operator: corev1.NodeSelectorOperator("BadOp"), + Values: []string{"linux"}, + }, + }, + }, + }, + }, + }, + } + err := ValidateAffinityRules(affinity, fldPath) + require.Error(t, err) + assert.Contains(t, err.Error(), "not a valid selector operator") + }) +} diff --git a/pkg/tlsprofile/tlsprofile_test.go b/pkg/tlsprofile/tlsprofile_test.go index 5778609c2..8fd50d121 100644 --- a/pkg/tlsprofile/tlsprofile_test.go +++ b/pkg/tlsprofile/tlsprofile_test.go @@ -77,6 +77,20 @@ func TestEffectiveSpec_custom(t *testing.T) { } } +func TestEffectiveSpec_customWithNilCustomReturnsError(t *testing.T) { + profile := &configv1.TLSSecurityProfile{ + Type: configv1.TLSProfileCustomType, + Custom: nil, + } + _, err := EffectiveSpec(profile) + if err == nil { + t.Fatal("expected error for custom type with nil custom settings") + } + if !strings.Contains(err.Error(), "missing custom settings") { + t.Fatalf("expected error about missing custom settings, got: %v", err) + } +} + func TestCertManagerWebhookTLSArgs_nilSpecReturnsEmpty(t *testing.T) { args := CertManagerWebhookTLSArgs(nil) if len(args) != 0 {