Skip to content

Commit

Permalink
feat: port name validation RFC-6335 (#2474)
Browse files Browse the repository at this point in the history
## Description
<!-- Describe this change, how it works, and the motivation behind it.
-->

Kubernetes adheres to
[RFC-6335](https://www.rfc-archive.org/getrfc.php?rfc=6335#gsc.tab=0),
which specifies that port names must be between 1 and 15 characters.
However, this constraint is currently not enforced by the Kurtosis
engine, leading to deployment failures in Kubernetes environments when
port names exceed 15 characters.

Here is an example that the Kurtosis validation step failed to catch:

![image
(1)](https://github.com/kurtosis-tech/kurtosis/assets/28714795/cb2f8ee4-038f-4368-9901-e5432fc58e45)


This PR resolves the issue by adding an additional validation step for
port names. It also refactors the regex definitions in `service.go` and
improves the test design for validating service and port names in
`service_test.go`.

## Is this change user facing?
YES

<!-- If yes, please add the "user facing" label to the PR -->
<!-- If yes, don't forget to include docs changes where relevant -->

## References (if applicable)
<!-- Add relevant Github Issues, Discord threads, or other helpful
information. -->
<!-- You can auto-close issues by putting "Fixes #XXXX" here. -->

-
https://github.com/kubernetes/kubernetes/blob/master/staging/src/k8s.io/apimachinery/pkg/util/validation/validation.go#L326-L351
- 0xPolygon/kurtosis-cdk#146 (comment)
  • Loading branch information
leovct authored Jun 6, 2024
1 parent 281ae3d commit ffbd30b
Show file tree
Hide file tree
Showing 3 changed files with 100 additions and 34 deletions.
Original file line number Diff line number Diff line change
@@ -1,31 +1,49 @@
package service

import (
"github.com/kurtosis-tech/kurtosis/container-engine-lib/lib/backend_interface/objects/container"
"github.com/kurtosis-tech/kurtosis/container-engine-lib/lib/backend_interface/objects/port_spec"
"net"
"regexp"
"strconv"

"github.com/kurtosis-tech/kurtosis/container-engine-lib/lib/backend_interface/objects/container"
"github.com/kurtosis-tech/kurtosis/container-engine-lib/lib/backend_interface/objects/port_spec"
)

const (
var (
// ServiceNameRegex implements RFC-1035 for naming services, namely:
// * contain at least 1 character
// * contain at most 63 characters
// * contain only lowercase alphanumeric characters or '-'
// * contain at least one letter ('A'-'Z' or 'a'-'z')
// * start with an alphabetic character
// * end with an alphanumeric character
// The adoption of RFC-1035 is to maintain compatability with current Kubernetes service and pod naming standards:
// We use this over RFC-1035 as Service Names require 1035 to be followed
// https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#dns-label-names
// https://kubernetes.io/docs/concepts/services-networking/service/
ServiceNameRegex = "[a-z]([-a-z0-9]{0,61}[a-z0-9])?"
WordWrappedServiceNameRegex = "^" + ServiceNameRegex + "$"
serviceNameMaxLength = 63
)
// nolint: gomnd
ServiceNameRegex = regexp.MustCompile(generateRegex(63))

var (
compiledWordWrappedServiceNameRegex = regexp.MustCompile(WordWrappedServiceNameRegex)
// PortNameRegex implements RFC-6335 for naming ports, namely:
// * contain at least 1 character
// * contain at most 15 characters
// * contain only lowercase alphanumeric characters or '-'
// * contain at least one letter ('A'-'Z' or 'a'-'z')
// * start with an alphabetic character
// * end with an alphanumeric character
// The adpoption of RFC-6335 is to maintain compatability with current Kubernetes port naming standards:
// We use this over RFC-6335 as Port Names require 6335 to be followed
// https://github.com/kubernetes/kubernetes/blob/master/staging/src/k8s.io/apimachinery/pkg/util/validation/validation.go#L326-L351
// nolint: gomnd
PortNameRegex = regexp.MustCompile(generateRegex(15))
)

// generateRegex creates a regex string based on the provided constraints (RFC-1035 and RFC-6335).
func generateRegex(maxLen int) string {
// nolint: gomnd
return "^[a-z]([-a-z0-9]{0," + strconv.Itoa(maxLen-2) + "}[a-z0-9])?$"
}

type ServiceName string
type ServiceUUID string

Expand Down Expand Up @@ -79,5 +97,9 @@ func (service *Service) GetMaybePublicPorts() map[string]*port_spec.PortSpec {
}

func IsServiceNameValid(serviceName ServiceName) bool {
return compiledWordWrappedServiceNameRegex.MatchString(string(serviceName))
return ServiceNameRegex.MatchString(string(serviceName))
}

func IsPortNameValid(portName string) bool {
return PortNameRegex.MatchString(portName)
}
Original file line number Diff line number Diff line change
@@ -1,34 +1,65 @@
package service

import (
"github.com/stretchr/testify/require"
"testing"

"github.com/stretchr/testify/require"
)

func TestValidServiceName(t *testing.T) {
require.True(t, IsServiceNameValid(ServiceName("a")))
require.True(t, IsServiceNameValid(ServiceName("abc")))
require.True(t, IsServiceNameValid(ServiceName("abcdefabcdefabcdefabcdefabcdefabcdefabcdefabcdefabcdefabcdef123")))
}
func TestServiceNameValidation(t *testing.T) {
validServiceNames := []string{
"a",
"abc",
"abcdefabcdefabcdefabcdefabcdefabcdefabcdefabcdefabcdefabcdef123", // 63 characters
"a-b",
}

func TestInvalidServiceName(t *testing.T) {
require.False(t, IsServiceNameValid(ServiceName("1-geth-lighthouse")))
}
invalidServiceNames := []string{
"1-geth-lighthouse", // 17 characters
"-bc",
"a--",
"a_b",
"a%b",
"a:b",
"a/b",
"",
"abcdefabcdefabcdefabcdefabcdefabcdefabcdefabcdefabcdefabcdef1234", // 64 characters
}

func TestServiceNameWithSpecialChars(t *testing.T) {
require.True(t, IsServiceNameValid(ServiceName("a-b")))
require.False(t, IsServiceNameValid(ServiceName("-bc")))
require.False(t, IsServiceNameValid(ServiceName("a--")))
require.False(t, IsServiceNameValid(ServiceName("a_b")))
require.False(t, IsServiceNameValid(ServiceName("a%b")))
require.False(t, IsServiceNameValid(ServiceName("a:b")))
require.False(t, IsServiceNameValid(ServiceName("a/b")))
for _, name := range validServiceNames {
require.True(t, IsServiceNameValid(ServiceName(name)), "expected valid service name: %s", name)
}

for _, name := range invalidServiceNames {
require.False(t, IsServiceNameValid(ServiceName(name)), "expected invalid service name: %s", name)
}
}

func TestServiceNameLength(t *testing.T) {
require.False(t, IsServiceNameValid(ServiceName("")))
require.True(t, IsServiceNameValid(ServiceName("a")))
require.True(t, IsServiceNameValid(ServiceName("abc")))
require.True(t, IsServiceNameValid(ServiceName("abcdefabcdefabcdefabcdefabcdefabcdefabcdefabcdefabcdefabcdef123")))
require.False(t, IsServiceNameValid(ServiceName("abcdefabcdefabcdefabcdefabcdefabcdefabcdefabcdefabcdefabcdef1234")))
func TestPortNameValidation(t *testing.T) {
validPortNames := []string{
"a",
"abc",
"abcdefabcdef123", // 15 characters
"a-b",
}

invalidPortNames := []string{
"1-dummy-port", // 12 characters
"-bc",
"a--",
"a_b",
"a%b",
"a:b",
"a/b",
"",
"abcdefabcdef1234", // 16 characters
}

for _, name := range validPortNames {
require.True(t, IsPortNameValid(name), "expected valid port name: %s", name)
}

for _, name := range invalidPortNames {
require.False(t, IsPortNameValid(name), "expected invalid port name: %s", name)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,9 @@ func validateSingleService(validatorEnvironment *startosis_validator.ValidatorEn

var portIds []string
for portId := range serviceConfig.GetPrivatePorts() {
if isValidPortName := service.IsPortNameValid(portId); !isValidPortName {
return startosis_errors.NewValidationError(invalidPortNameErrorText(portId))
}
portIds = append(portIds, portId)
}
validatorEnvironment.AddPrivatePortIDForService(portIds, serviceName)
Expand All @@ -131,7 +134,17 @@ func invalidServiceNameErrorText(
return fmt.Sprintf(
"Service name '%v' is invalid as it contains disallowed characters. Service names must adhere to the RFC 1035 standard, specifically implementing this regex and be 1-63 characters long: %s. This means the service name must only contain lowercase alphanumeric characters or '-', and must start with a lowercase alphabet and end with a lowercase alphanumeric character.",
serviceName,
service.WordWrappedServiceNameRegex,
service.ServiceNameRegex,
)
}

func invalidPortNameErrorText(
portName string,
) string {
return fmt.Sprintf(
"Port name '%v' is invalid as it contains disallowed characters. Service names must adhere to the RFC 6335 standard, specifically implementing this regex and be 1-15 characters long: %s. This means the service name must only contain lowercase alphanumeric characters or '-', and must start with a lowercase alphabet and end with a lowercase alphanumeric character.",
portName,
service.PortNameRegex,
)
}

Expand Down

0 comments on commit ffbd30b

Please sign in to comment.