feat(vol): move validation to webhook

apis/v1alpha1/instrumentation_webhook.go

@@ -17,6 +17,7 @@ package v1alpha1
 import (
 	"context"
 	"fmt"
+	"reflect"
 	"strconv"
 	"strings"
 
@@ -236,6 +237,37 @@ func (w InstrumentationWebhook) validate(r *Instrumentation) (admission.Warnings
 	default:
 		return warnings, fmt.Errorf("spec.sampler.type is not valid: %s", r.Spec.Sampler.Type)
 	}
+
+	var err error
+	err = validateInstrVolume(r.Spec.ApacheHttpd.Volume, r.Spec.ApacheHttpd.VolumeSizeLimit)
+	if err != nil {
+		return warnings, fmt.Errorf("spec.apachehttpd.volume and spec.apachehttpd.volumeSizeLimit cannot both be defined: %w", err)
+	}
+	err = validateInstrVolume(r.Spec.DotNet.Volume, r.Spec.DotNet.VolumeSizeLimit)
+	if err != nil {
+		return warnings, fmt.Errorf("spec.dotnet.volume and spec.dotnet.volumeSizeLimit cannot both be defined: %w", err)
+	}
+	err = validateInstrVolume(r.Spec.Go.Volume, r.Spec.Go.VolumeSizeLimit)
+	if err != nil {
+		return warnings, fmt.Errorf("spec.go.volume and spec.go.volumeSizeLimit cannot both be defined: %w", err)
+	}
+	err = validateInstrVolume(r.Spec.Java.Volume, r.Spec.Java.VolumeSizeLimit)
+	if err != nil {
+		return warnings, fmt.Errorf("spec.java.volume and spec.java.volumeSizeLimit cannot both be defined: %w", err)
+	}
+	err = validateInstrVolume(r.Spec.Nginx.Volume, r.Spec.Nginx.VolumeSizeLimit)
+	if err != nil {
+		return warnings, fmt.Errorf("spec.nginx.volume and spec.nginx.volumeSizeLimit cannot both be defined: %w", err)
+	}
+	err = validateInstrVolume(r.Spec.NodeJS.Volume, r.Spec.NodeJS.VolumeSizeLimit)
+	if err != nil {
+		return warnings, fmt.Errorf("spec.nodejs.volume and spec.nodejs.volumeSizeLimit cannot both be defined: %w", err)
+	}
+	err = validateInstrVolume(r.Spec.Python.Volume, r.Spec.Python.VolumeSizeLimit)
+	if err != nil {
+		return warnings, fmt.Errorf("spec.python.volume and spec.python.volumeSizeLimit cannot both be defined: %w", err)
+	}
+
 	return warnings, nil
 }
 
@@ -270,6 +302,13 @@ func validateJaegerRemoteSamplerArgument(argument string) error {
 	return nil
 }
 
+func validateInstrVolume(volume corev1.Volume, volumeSizeLimit *resource.Quantity) error {
+	if !reflect.ValueOf(volume).IsZero() && volumeSizeLimit != nil {
+		return fmt.Errorf("unable to resolve volume size")
+	}
+	return nil
+}
+
 func NewInstrumentationWebhook(logger logr.Logger, scheme *runtime.Scheme, cfg config.Config) *InstrumentationWebhook {
 	return &InstrumentationWebhook{
 		logger: logger,

pkg/instrumentation/apachehttpd.go

@@ -61,11 +61,11 @@ const (
 
 func injectApacheHttpdagent(_ logr.Logger, apacheSpec v1alpha1.ApacheHttpd, pod corev1.Pod, index int, otlpEndpoint string, resourceMap map[string]string) corev1.Pod {
 
+	volume := instrVolume(apacheSpec.Volume, apacheAgentVolume, apacheSpec.VolumeSizeLimit)
+
 	// caller checks if there is at least one container
 	container := &pod.Spec.Containers[index]
 
-	volume, _ := instrVolume(apacheSpec.Volume, apacheAgentVolume, apacheSpec.VolumeSizeLimit)
-
 	// inject env vars
 	for _, env := range apacheSpec.Env {
 		idx := getIndexOfEnv(container.Env, env.Name)

pkg/instrumentation/dotnet.go

@@ -52,6 +52,8 @@ const (
 
 func injectDotNetSDK(dotNetSpec v1alpha1.DotNet, pod corev1.Pod, index int, runtime string) (corev1.Pod, error) {
 
+	volume := instrVolume(dotNetSpec.Volume, dotnetVolumeName, dotNetSpec.VolumeSizeLimit)
+
 	// caller checks if there is at least one container.
 	container := &pod.Spec.Containers[index]
 
@@ -60,11 +62,6 @@ func injectDotNetSDK(dotNetSpec v1alpha1.DotNet, pod corev1.Pod, index int, runt
 		return pod, err
 	}
 
-	volume, err := instrVolume(dotNetSpec.Volume, dotnetVolumeName, dotNetSpec.VolumeSizeLimit)
-	if err != nil {
-		return pod, err
-	}
-
 	// check if OTEL_DOTNET_AUTO_HOME env var is already set in the container
 	// if it is already set, then we assume that .NET Auto-instrumentation is already configured for this container
 	if getIndexOfEnv(container.Env, envDotNetOTelAutoHome) > -1 {

pkg/instrumentation/helper.go

@@ -124,20 +124,19 @@ func isInstrWithoutContainers(inst instrumentationWithContainers) int {
 	return 0
 }
 
-func instrVolume(volume corev1.Volume, name string, volumeSizeLimit *resource.Quantity) (corev1.Volume, error) {
-	if !reflect.ValueOf(volume).IsZero() && volumeSizeLimit != nil {
-		return volume, fmt.Errorf("both Volume and VolumeSizeLimit cannot be defined simultaneously")
-	} else if !reflect.ValueOf(volume).IsZero() {
-		return volume, nil
+// Return volume if defined, otherwise return emptyDir with given name and size limit.
+func instrVolume(volume corev1.Volume, name string, quantity *resource.Quantity) corev1.Volume {
+	if !reflect.ValueOf(volume).IsZero() {
+		return volume
 	}
 
 	return corev1.Volume{
 		Name: name,
 		VolumeSource: corev1.VolumeSource{
 			EmptyDir: &corev1.EmptyDirVolumeSource{
-				SizeLimit: volumeSize(volumeSizeLimit),
+				SizeLimit: volumeSize(quantity),
 			},
-		}}, nil
+		}}
 }
 
 func volumeSize(quantity *resource.Quantity) *resource.Quantity {

pkg/instrumentation/helper_test.go

@@ -191,12 +191,11 @@ func TestDuplicatedContainers(t *testing.T) {
 
 func TestInstrVolume(t *testing.T) {
 	tests := []struct {
-		name            string
-		volume          corev1.Volume
-		volumeName      string
-		volumeSizeLimit *resource.Quantity
-		expected        corev1.Volume
-		err             error
+		name       string
+		volume     corev1.Volume
+		volumeName string
+		quantity   *resource.Quantity
+		expected   corev1.Volume
 	}{
 		{
 			name: "With volume",
@@ -207,44 +206,41 @@ func TestInstrVolume(t *testing.T) {
 						SizeLimit: &resource.Quantity{},
 					},
 				}},
-			volumeName:      "default-vol",
-			volumeSizeLimit: nil,
+			volumeName: "default-vol",
+			quantity:   nil,
 			expected: corev1.Volume{
 				Name: "vol1",
 				VolumeSource: corev1.VolumeSource{
 					EmptyDir: &corev1.EmptyDirVolumeSource{
 						SizeLimit: &resource.Quantity{},
 					},
 				}},
-			err: nil,
 		},
 		{
-			name:            "With volume size limit",
-			volume:          corev1.Volume{},
-			volumeName:      "default-vol",
-			volumeSizeLimit: &defaultVolumeLimitSize,
+			name:       "With volume size limit",
+			volume:     corev1.Volume{},
+			volumeName: "default-vol",
+			quantity:   &defaultVolumeLimitSize,
 			expected: corev1.Volume{
 				Name: "default-vol",
 				VolumeSource: corev1.VolumeSource{
 					EmptyDir: &corev1.EmptyDirVolumeSource{
 						SizeLimit: &defaultVolumeLimitSize,
 					},
 				}},
-			err: nil,
 		},
 		{
-			name:            "No volume or size limit",
-			volume:          corev1.Volume{},
-			volumeName:      "default-vol",
-			volumeSizeLimit: nil,
+			name:       "No volume or size limit",
+			volume:     corev1.Volume{},
+			volumeName: "default-vol",
+			quantity:   nil,
 			expected: corev1.Volume{
 				Name: "default-vol",
 				VolumeSource: corev1.VolumeSource{
 					EmptyDir: &corev1.EmptyDirVolumeSource{
 						SizeLimit: &defaultSize,
 					},
 				}},
-			err: nil,
 		},
 		{
 			name: "With volume and size limit",
@@ -255,24 +251,22 @@ func TestInstrVolume(t *testing.T) {
 						SizeLimit: &resource.Quantity{},
 					},
 				}},
-			volumeName:      "default-vol",
-			volumeSizeLimit: &defaultVolumeLimitSize,
+			volumeName: "default-vol",
+			quantity:   &defaultVolumeLimitSize,
 			expected: corev1.Volume{
 				Name: "vol1",
 				VolumeSource: corev1.VolumeSource{
 					EmptyDir: &corev1.EmptyDirVolumeSource{
 						SizeLimit: &resource.Quantity{},
 					},
 				}},
-			err: fmt.Errorf("both Volume and VolumeSizeLimit cannot be defined simultaneously"),
 		},
 	}
 
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			res, err := instrVolume(test.volume, test.volumeName, test.volumeSizeLimit)
+			res := instrVolume(test.volume, test.volumeName, test.quantity)
 			assert.Equal(t, test.expected, res)
-			assert.Equal(t, test.err, err)
 		})
 	}
 }

pkg/instrumentation/javaagent.go

@@ -31,6 +31,8 @@ const (
 )
 
 func injectJavaagent(javaSpec v1alpha1.Java, pod corev1.Pod, index int) (corev1.Pod, error) {
+	volume := instrVolume(javaSpec.Volume, javaVolumeName, javaSpec.VolumeSizeLimit)
+
 	// caller checks if there is at least one container.
 	container := &pod.Spec.Containers[index]
 
@@ -39,11 +41,6 @@ func injectJavaagent(javaSpec v1alpha1.Java, pod corev1.Pod, index int) (corev1.
 		return pod, err
 	}
 
-	volume, err := instrVolume(javaSpec.Volume, javaVolumeName, javaSpec.VolumeSizeLimit)
-	if err != nil {
-		return pod, err
-	}
-
 	// inject Java instrumentation spec env vars.
 	for _, env := range javaSpec.Env {
 		idx := getIndexOfEnv(container.Env, env.Name)

pkg/instrumentation/nodejs.go

@@ -29,6 +29,8 @@ const (
 )
 
 func injectNodeJSSDK(nodeJSSpec v1alpha1.NodeJS, pod corev1.Pod, index int) (corev1.Pod, error) {
+	volume := instrVolume(nodeJSSpec.Volume, nodejsVolumeName, nodeJSSpec.VolumeSizeLimit)
+
 	// caller checks if there is at least one container.
 	container := &pod.Spec.Containers[index]
 
@@ -37,11 +39,6 @@ func injectNodeJSSDK(nodeJSSpec v1alpha1.NodeJS, pod corev1.Pod, index int) (cor
 		return pod, err
 	}
 
-	volume, err := instrVolume(nodeJSSpec.Volume, nodejsVolumeName, nodeJSSpec.VolumeSizeLimit)
-	if err != nil {
-		return pod, err
-	}
-
 	// inject NodeJS instrumentation spec env vars.
 	for _, env := range nodeJSSpec.Env {
 		idx := getIndexOfEnv(container.Env, env.Name)

pkg/instrumentation/python.go

@@ -35,6 +35,8 @@ const (
 )
 
 func injectPythonSDK(pythonSpec v1alpha1.Python, pod corev1.Pod, index int) (corev1.Pod, error) {
+	volume := instrVolume(pythonSpec.Volume, pythonVolumeName, pythonSpec.VolumeSizeLimit)
+
 	// caller checks if there is at least one container.
 	container := &pod.Spec.Containers[index]
 
@@ -43,11 +45,6 @@ func injectPythonSDK(pythonSpec v1alpha1.Python, pod corev1.Pod, index int) (cor
 		return pod, err
 	}
 
-	volume, err := instrVolume(pythonSpec.Volume, pythonVolumeName, pythonSpec.VolumeSizeLimit)
-	if err != nil {
-		return pod, err
-	}
-
 	// inject Python instrumentation spec env vars.
 	for _, env := range pythonSpec.Env {
 		idx := getIndexOfEnv(container.Env, env.Name)