Skip to content

Commit

Permalink
Merge pull request #8280 from killianmuldoon/pr-runtimesdk-validate-h…
Browse files Browse the repository at this point in the history
…ooknam

🌱 Update handler name validation rules
  • Loading branch information
k8s-ci-robot authored Mar 14, 2023
2 parents 821b957 + a57e32b commit 88d47b2
Show file tree
Hide file tree
Showing 5 changed files with 38 additions and 3 deletions.
1 change: 1 addition & 0 deletions exp/runtime/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ type ExtensionHandler struct {
Hook runtimecatalog.Hook

// Name is the name of the extension handler.
// An extension handler name must be valid in line RFC 1123 Label Names.
Name string

// HandlerFunc is the handler function.
Expand Down
4 changes: 2 additions & 2 deletions internal/runtime/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -586,8 +586,8 @@ func defaultAndValidateDiscoveryResponse(cat *runtimecatalog.Catalog, discovery
}
names[handler.Name] = true

// Name should match Kubernetes naming conventions - validated based on Kubernetes DNS1123 Subdomain rules.
if errStrings := validation.IsDNS1123Subdomain(handler.Name); len(errStrings) > 0 {
// Name should match Kubernetes naming conventions - validated based on DNS1123 label rules.
if errStrings := validation.IsDNS1123Label(handler.Name); len(errStrings) > 0 {
errs = append(errs, errors.Errorf("handler name %s is not valid: %s", handler.Name, errStrings))
}

Expand Down
19 changes: 18 additions & 1 deletion internal/runtime/client/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,7 @@ func Test_defaultAndValidateDiscoveryResponse(t *testing.T) {
wantErr: false,
},
{
name: "error with name violating DNS1123",
name: "error if handler name has capital letters",
discovery: &runtimehooksv1.DiscoveryResponse{
TypeMeta: metav1.TypeMeta{
Kind: "DiscoveryResponse",
Expand All @@ -378,6 +378,23 @@ func Test_defaultAndValidateDiscoveryResponse(t *testing.T) {
},
wantErr: true,
},
{
name: "error if handler name has full stops",
discovery: &runtimehooksv1.DiscoveryResponse{
TypeMeta: metav1.TypeMeta{
Kind: "DiscoveryResponse",
APIVersion: runtimehooksv1.GroupVersion.String(),
},
Handlers: []runtimehooksv1.ExtensionHandler{{
Name: "has.full.stops",
RequestHook: runtimehooksv1.GroupVersionHook{
Hook: "FakeHook",
APIVersion: fakev1alpha1.GroupVersion.String(),
},
}},
},
wantErr: true,
},
{
name: "error with Timeout of over 30 seconds",
discovery: &runtimehooksv1.DiscoveryResponse{
Expand Down
8 changes: 8 additions & 0 deletions internal/webhooks/runtime/extensionconfig_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,14 @@ func (webhook *ExtensionConfig) validate(_ context.Context, _, newExtensionConfi
}

var allErrs field.ErrorList

// Name should match Kubernetes naming conventions - validated based on DNS1123 label rules.
if errStrings := validation.IsDNS1123Label(newExtensionConfig.Name); len(errStrings) > 0 {
allErrs = append(allErrs, field.Invalid(
field.NewPath("metadata", "name"),
newExtensionConfig.Name,
fmt.Sprintf("ExtensionConfig name should be a valid DNS1123 label name: %s", errStrings)))
}
allErrs = append(allErrs, validateExtensionConfigSpec(newExtensionConfig)...)

if len(allErrs) > 0 {
Expand Down
9 changes: 9 additions & 0 deletions internal/webhooks/runtime/extensionconfig_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,9 @@ func TestExtensionConfigValidate(t *testing.T) {
extensionWithServiceAndURL := extensionWithURL.DeepCopy()
extensionWithServiceAndURL.Spec.ClientConfig.Service = extensionWithService.Spec.ClientConfig.Service

extensionWithBadName := extensionWithURL.DeepCopy()
extensionWithBadName.Name = "bad.name"

// Valid updated Extension
updatedExtension := extensionWithURL.DeepCopy()
updatedExtension.Spec.ClientConfig.URL = pointer.String("https://a-in-extension-address.com")
Expand Down Expand Up @@ -238,6 +241,12 @@ func TestExtensionConfigValidate(t *testing.T) {
featureGate: true,
expectErr: true,
},
{
name: "creation should fail if extensionConfig Name violates Kubernetes naming rules",
in: extensionWithBadName,
featureGate: true,
expectErr: true,
},
{
name: "creation should fail if Service Name violates Kubernetes naming rules",
in: extensionWithBadServiceName,
Expand Down

0 comments on commit 88d47b2

Please sign in to comment.