Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions internal/webhook/scheduledsparkapplication_validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func (v *ScheduledSparkApplicationValidator) ValidateCreate(ctx context.Context,
return nil, nil
}
logger.Info("Validating SchedulingSparkApplication create", "name", app.Name, "namespace", app.Namespace)
if err := v.validate(app); err != nil {
if err := v.validate(ctx, app); err != nil {
return nil, err
}
return nil, nil
Expand All @@ -58,7 +58,7 @@ func (v *ScheduledSparkApplicationValidator) ValidateUpdate(ctx context.Context,
return nil, nil
}
logger.Info("Validating SchedulingSparkApplication update", "name", newApp.Name, "namespace", newApp.Namespace)
if err := v.validate(newApp); err != nil {
if err := v.validate(ctx, newApp); err != nil {
return nil, err
}
return nil, nil
Expand All @@ -74,7 +74,11 @@ func (v *ScheduledSparkApplicationValidator) ValidateDelete(ctx context.Context,
return nil, nil
}

func (v *ScheduledSparkApplicationValidator) validate(_ *v1beta2.ScheduledSparkApplication) error {
// TODO: implement validate logic
func (v *ScheduledSparkApplicationValidator) validate(ctx context.Context, app *v1beta2.ScheduledSparkApplication) error {
meta := app.ObjectMeta
err := validateNameLength(ctx, meta)
if err != nil {
return err
}
return nil
}
25 changes: 25 additions & 0 deletions internal/webhook/sparkapplication_validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/equality"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/util/validation"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"

Expand All @@ -34,6 +35,11 @@ import (
// Modifying the path for an invalid path can cause API server errors; failing to locate the webhook.
// +kubebuilder:webhook:admissionReviewVersions=v1,failurePolicy=fail,groups=sparkoperator.k8s.io,matchPolicy=Exact,mutating=false,name=validate-sparkapplication.sparkoperator.k8s.io,path=/validate-sparkoperator-k8s-io-v1beta2-sparkapplication,reinvocationPolicy=Never,resources=sparkapplications,sideEffects=None,verbs=create;update,versions=v1beta2,webhookVersions=v1

const (
// 63 (Kubernetes limit) - 10 (max suffix length, e.g., "-driver-svc")
maxAppNameLength = 53
)

type SparkApplicationValidator struct {
client client.Client

Expand All @@ -58,6 +64,11 @@ func (v *SparkApplicationValidator) ValidateCreate(ctx context.Context, obj runt
return nil, nil
}
logger.Info("Validating SparkApplication create", "name", app.Name, "namespace", app.Namespace, "state", util.GetApplicationState(app))

if err := v.validateMetadata(ctx, app); err != nil {
return nil, err
}

if err := v.validateSpec(ctx, app); err != nil {
return nil, err
}
Expand Down Expand Up @@ -185,3 +196,17 @@ func (v *SparkApplicationValidator) validateResourceUsage(ctx context.Context, a

return nil
}

func (v *SparkApplicationValidator) validateMetadata(ctx context.Context, app *v1beta2.SparkApplication) error {
meta := app.ObjectMeta
if err := validateNameLength(ctx, meta); err != nil {
return err
}

svcName := util.GetDefaultUIServiceName(app)
if errs := validation.IsDNS1035Label(svcName); len(errs) != 0 {
return fmt.Errorf("spark UI service name %q is not a DNS 1035 label. Try to make the application name shorter", svcName)
}

return nil
}
22 changes: 22 additions & 0 deletions internal/webhook/webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,12 @@ limitations under the License.
package webhook

import (
"context"
"fmt"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/validation"
"k8s.io/apimachinery/pkg/util/validation/field"
ctrl "sigs.k8s.io/controller-runtime"
)

Expand All @@ -35,3 +41,19 @@ type Options struct {
WebhookMetricsBindAddress string
EnableResourceQuotaEnforcement bool
}

// validateNameLength checks if the application name exceeds the limit for Kubernetes labels.
// The RFC 1123 DNS label regex is handled by Kubernetes itself,
// we only need to check the length here to provide a more specific
// and early error message.
func validateNameLength(_ context.Context, appMeta metav1.ObjectMeta) error {
var allErrs field.ErrorList
if len(appMeta.Name) > validation.LabelValueMaxLength {
allErrs = append(allErrs, field.Invalid(field.NewPath("metadata", "name"), appMeta.Name,
fmt.Sprintf("name length must not exceed %d characters to allow for resource suffixes", maxAppNameLength)))
}
if allErrs != nil {
return allErrs.ToAggregate()
}
return nil
}
80 changes: 80 additions & 0 deletions internal/webhook/webhook_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
package webhook

import (
"context"
"fmt"
"strings"
"testing"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

// Note: The maxAppNameLength constant must also be defined in your main code.
// We define it here for the test. The value 63 is a common length
// limit for labels in Kubernetes.

func TestValidateNameLength(t *testing.T) {
// Define test cases
testCases := []struct {
name string
appName string
expectError bool
errorMsg string
}{
{
name: "name with valid length",
appName: "my-awesome-application",
expectError: false,
},
{
name: "empty name",
appName: "",
expectError: false, // Length is 0, which is less than the limit
},
{
name: "name at max length boundary",
appName: strings.Repeat("a", maxAppNameLength),
expectError: false,
},
{
name: "name exceeding max length by 1 character",
appName: strings.Repeat("a", maxAppNameLength+1),
expectError: true,
errorMsg: fmt.Sprintf("metadata.name: Invalid value: %q: name must be no more than %d characters to allow for resource suffixes", strings.Repeat("a", maxAppNameLength+1), maxAppNameLength),
},
{
name: "long name significantly over the limit",
appName: "this-is-a-very-very-very-long-application-name-that-definitely-exceeds-the-kubernetes-label-length-limit",
expectError: true,
errorMsg: fmt.Sprintf("metadata.name: Invalid value: %q: name must be no more than %d characters to allow for resource suffixes", "this-is-a-very-very-very-long-application-name-that-definitely-exceeds-the-kubernetes-label-length-limit", maxAppNameLength),
},
}

// Run tests in a loop
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
// Prepare test data
appMeta := metav1.ObjectMeta{
Name: tc.appName,
}

// Call the function under test
err := validateNameLength(context.Background(), appMeta)

// Check the result
if tc.expectError {
if err == nil {
t.Errorf("expected an error but got nil")
return
}
if err.Error() != tc.errorMsg {
t.Errorf("expected error message:\n%q\ngot:\n%q", tc.errorMsg, err.Error())
}
} else {
if err != nil {
t.Errorf("unexpected error: %v", err)
}
}
})
}
}
26 changes: 26 additions & 0 deletions test/e2e/bad_examples/long-name-validation.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
apiVersion: sparkoperator.k8s.io/v1beta2
kind: SparkApplication
metadata:
name: this-is-a-very-very-very-long-application-name-that-words
spec:
type: Scala
mode: cluster
image: docker.io/library/spark:4.0.0
imagePullPolicy: IfNotPresent
mainClass: org.apache.spark.examples.SparkPi
mainApplicationFile: local:///opt/spark/examples/jars/spark-examples.jar
arguments:
- "5000"
sparkVersion: 4.0.0
driver:
labels:
version: 4.0.0
cores: 1
memory: 512m
serviceAccount: spark-operator-spark
executor:
labels:
version: 4.0.0
instances: 1
cores: 1
memory: 512m
24 changes: 24 additions & 0 deletions test/e2e/sparkapplication_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -393,4 +393,28 @@ var _ = Describe("Example SparkApplication", func() {
Expect(strings.Contains(string(bytes), "Pi is roughly 3")).To(BeTrue())
})
})

Context("long-name-validation", func() {
ctx := context.Background()
path := filepath.Join("bad_examples", "long-name-validation.yaml")
app := &v1beta2.SparkApplication{}

BeforeEach(func() {
By("Parsing SparkApplication from file")
file, err := os.Open(path)
Expect(err).NotTo(HaveOccurred())
Expect(file).NotTo(BeNil())

decoder := yaml.NewYAMLOrJSONDecoder(file, 100)
Expect(decoder).NotTo(BeNil())
Expect(decoder.Decode(app)).NotTo(HaveOccurred())
})

It("Should be rejected by the webhook due to name length exceeding the limit", func() {
By("Attempting to create SparkApplication with a name that exceeds the maximum length")
err := k8sClient.Create(ctx, app)
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring("name must be no more than 53 characters"))
})
})
})
Loading