Skip to content

Commit

Permalink
fix: Validate workspace name (#726)
Browse files Browse the repository at this point in the history
**Reason for Change**:
<!-- What does this PR improve or fix in Kaito? Why is it needed? -->

check if it is a valid DNSLabel

**Requirements**

- [x] added unit tests and e2e tests (if applicable).

**Issue Fixed**:
<!-- If this PR fixes GitHub issue 4321, add "Fixes #4321" to the next
line. -->

Fixes #725 

**Notes for Reviewers**:

---------

Signed-off-by: jerryzhuang <zhuangqhc@gmail.com>
  • Loading branch information
zhuangqh authored Nov 28, 2024
1 parent 2cb5710 commit 3dbb660
Show file tree
Hide file tree
Showing 2 changed files with 97 additions and 12 deletions.
17 changes: 7 additions & 10 deletions api/v1alpha1/workspace_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
admissionregistrationv1 "k8s.io/api/admissionregistration/v1"
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/validation"
"k8s.io/klog/v2"
"knative.dev/pkg/apis"
)
Expand All @@ -41,6 +42,10 @@ func (w *Workspace) SupportedVerbs() []admissionregistrationv1.OperationType {
}

func (w *Workspace) Validate(ctx context.Context) (errs *apis.FieldError) {
errmsgs := validation.IsDNS1123Label(w.Name)
if len(errmsgs) > 0 {
errs = errs.Also(apis.ErrInvalidValue(strings.Join(errmsgs, ", "), "name"))
}
base := apis.GetBaseline(ctx)
if base == nil {
klog.InfoS("Validate creation", "workspace", fmt.Sprintf("%s/%s", w.Namespace, w.Name))
Expand Down Expand Up @@ -93,14 +98,6 @@ func (w *Workspace) validateUpdate(old *Workspace) (errs *apis.FieldError) {
return errs
}

func ValidateDNSSubdomain(name string) bool {
var dnsSubDomainRegexp = regexp.MustCompile(`^(?i:[a-z0-9]([-a-z0-9]*[a-z0-9])?)$`)
if len(name) < 1 || len(name) > 253 {
return false
}
return dnsSubDomainRegexp.MatchString(name)
}

func (r *AdapterSpec) validateCreateorUpdate() (errs *apis.FieldError) {
if r.Source == nil {
errs = errs.Also(apis.ErrMissingField("Source"))
Expand All @@ -109,8 +106,8 @@ func (r *AdapterSpec) validateCreateorUpdate() (errs *apis.FieldError) {

if r.Source.Name == "" {
errs = errs.Also(apis.ErrMissingField("Name of Adapter field must be specified"))
} else if !ValidateDNSSubdomain(r.Source.Name) {
errs = errs.Also(apis.ErrMissingField("Name of Adapter must be a valid DNS subdomain value"))
} else if errmsgs := validation.IsDNS1123Subdomain(r.Source.Name); len(errmsgs) > 0 {
errs = errs.Also(apis.ErrInvalidValue(strings.Join(errmsgs, ", "), "adapters.source.name"))
}
if r.Source.Image == "" {
errs = errs.Also(apis.ErrMissingField("Image of Adapter field must be specified"))
Expand Down
92 changes: 90 additions & 2 deletions api/v1alpha1/workspace_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,29 @@ func (*testModel) SupportTuning() bool {
return true
}

type testModelStatic struct{}

func (*testModelStatic) GetInferenceParameters() *model.PresetParam {
return &model.PresetParam{
GPUCountRequirement: "1",
TotalGPUMemoryRequirement: "16Gi",
PerGPUMemoryRequirement: "16Gi",
}
}
func (*testModelStatic) GetTuningParameters() *model.PresetParam {
return &model.PresetParam{
GPUCountRequirement: "1",
TotalGPUMemoryRequirement: "16Gi",
PerGPUMemoryRequirement: "16Gi",
}
}
func (*testModelStatic) SupportDistributedInference() bool {
return false
}
func (*testModelStatic) SupportTuning() bool {
return true
}

type testModelPrivate struct{}

func (*testModelPrivate) GetInferenceParameters() *model.PresetParam {
Expand Down Expand Up @@ -91,6 +114,7 @@ func (*testModelPrivate) SupportTuning() bool {
func RegisterValidationTestModels() {
var test testModel
var testPrivate testModelPrivate
var testStatic testModelStatic
plugin.KaitoModelRegister.Register(&plugin.Registration{
Name: "test-validation",
Instance: &test,
Expand All @@ -99,6 +123,10 @@ func RegisterValidationTestModels() {
Name: "private-test-validation",
Instance: &testPrivate,
})
plugin.KaitoModelRegister.Register(&plugin.Registration{
Name: "test-validation-static",
Instance: &testStatic,
})
}

func pointerToInt(i int) *int {
Expand Down Expand Up @@ -703,14 +731,14 @@ func TestAdapterSpecValidateCreateorUpdate(t *testing.T) {
},
Strength: &ValidStrength,
},
errContent: "Name of Adapter must be a valid DNS subdomain value",
errContent: "invalid value",
expectErrs: true,
},
{
name: "Valid Adapter",
adapterSpec: &AdapterSpec{
Source: &DataSource{
Name: "Adapter-1",
Name: "adapter-1",
Image: "fake.kaito.com/kaito-image:0.0.1",
},
},
Expand Down Expand Up @@ -898,6 +926,66 @@ func TestWorkspaceValidateCreate(t *testing.T) {
}
}

func TestWorkspaceValidateName(t *testing.T) {
testWorkspace := &Workspace{
ObjectMeta: metav1.ObjectMeta{
Name: "testWorkspace",
Namespace: "kaito",
},
Resource: ResourceSpec{
InstanceType: "Standard_NC12s_v3",
Count: pointerToInt(1),
},
Inference: &InferenceSpec{
Preset: &PresetSpec{
PresetMeta: PresetMeta{
Name: ModelName("test-validation-static"),
},
},
},
}
RegisterValidationTestModels()
os.Setenv("CLOUD_PROVIDER", consts.AzureCloudName)
tests := []struct {
name string
workspaceName string
wantErr bool
errField string
}{
{
name: "Valid name",
workspaceName: "valid-name",
wantErr: false,
},
{
name: "Name with invdalid characters",
workspaceName: "phi-3.5-mini",
wantErr: true,
errField: "name",
},
{
name: "Name start with invalid character",
workspaceName: "-mini",
wantErr: true,
errField: "name",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
workspace := testWorkspace.DeepCopy()
workspace.Name = tt.workspaceName
errs := workspace.Validate(context.Background())
if (errs != nil) != tt.wantErr {
t.Errorf("Validate() error = %v, wantErr %v", errs, tt.wantErr)
}
if errs != nil && !strings.Contains(errs.Error(), tt.errField) {
t.Errorf("Validate() expected error to contain field %s, but got %s", tt.errField, errs.Error())
}
})
}
}

func TestWorkspaceValidateUpdate(t *testing.T) {
tests := []struct {
name string
Expand Down

0 comments on commit 3dbb660

Please sign in to comment.