Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat!: Applying RFC-1123 standard to service names #749

Merged
merged 10 commits into from
Jun 21, 2023
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,20 @@ import (
)

const (
ServiceNameRegex = "[a-zA-Z0-9-_]+"
wordWrappedServiceNameRegex = "^" + ServiceNameRegex + "$"
// ServiceNameRegex implements RFC-1123 for naming services, namely:
// * contain at most 63 characters
// * contain only lowercase alphanumeric characters or '-'
// * start with an alphanumeric character
// * end with an alphanumeric character
// The adoption of RFC-1123 is to maintain compatability with current Kubernetes service and pod naming standards:
// https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#dns-label-names
ServiceNameRegex = "[a-z0-9]([-a-z0-9]*[a-z0-9])?"
adschwartz marked this conversation as resolved.
Show resolved Hide resolved
WordWrappedServiceNameRegex = "^" + ServiceNameRegex + "$"
serviceNameMaxLength = 63
)

var (
compiledWordWrappedServiceNameRegex = regexp.MustCompile(wordWrappedServiceNameRegex)
compiledWordWrappedServiceNameRegex = regexp.MustCompile(WordWrappedServiceNameRegex)
)

type ServiceName string
Expand Down Expand Up @@ -68,5 +76,10 @@ func (service *Service) GetMaybePublicPorts() map[string]*port_spec.PortSpec {
}

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

func isServiceNameCorrectLength(name ServiceName) bool {
return len(string(name)) <= serviceNameMaxLength && len(string(name)) > 0
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
package service

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

func TestValidServiceName(t *testing.T) {
require.True(t, IsServiceNameValid(ServiceName("a")))
require.True(t, IsServiceNameValid(ServiceName("abc")))
require.True(t, IsServiceNameValid(ServiceName("abcdefabcdefabcdefabcdefabcdefabcdefabcdefabcdefabcdefabcdef123")))
}

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")))
}

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")))
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func NewAddService(serviceNetwork service_network.ServiceNetwork, runtimeValueSt
IsOptional: false,
ZeroValueProvider: builtin_argument.ZeroValueProvider[starlark.String],
Validator: func(value starlark.Value) *startosis_errors.InterpretationError {
return builtin_argument.NonEmptyString(value, ServiceNameArgName)
adschwartz marked this conversation as resolved.
Show resolved Hide resolved
return validateServiceName(value, ServiceNameArgName)
},
},
{
Expand Down Expand Up @@ -168,3 +168,25 @@ func validateAndConvertConfigAndReadyCondition(

return apiServiceConfig, readyCondition, nil
}

func validateServiceName(
value starlark.Value,
serviceNameArgName string,
) *startosis_errors.InterpretationError {
e := builtin_argument.NonEmptyString(value, serviceNameArgName)
if e == nil {
valueStr, ok := value.(starlark.String)
if !ok {
return startosis_errors.NewInterpretationError("Expected a 'starlark.String', got '%s'", reflect.TypeOf(value))
}
serviceName := service.ServiceName(valueStr)
if !(service.IsServiceNameValid(serviceName)) {
return startosis_errors.NewInterpretationError(invalidServiceNameErrorText(serviceName))
} else {
return nil
}

} else {
return e
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ func validateSingleService(validatorEnvironment *startosis_validator.ValidatorEn
}
}
if isValidServiceName := service.IsServiceNameValid(serviceName); !isValidServiceName {
return startosis_errors.NewValidationError("Service name '%v' is invalid as it contains disallowed characters. Service names can only contain characters 'a-z', 'A-Z', '0-9', '-' & '_'", serviceName)
return startosis_errors.NewValidationError(invalidServiceNameErrorText(serviceName))
}

if validatorEnvironment.DoesServiceNameExist(serviceName) {
Expand All @@ -85,6 +85,16 @@ func validateSingleService(validatorEnvironment *startosis_validator.ValidatorEn
return nil
}

func invalidServiceNameErrorText(
serviceName service.ServiceName,
) string {
return fmt.Sprintf(
"Service name '%v' is invalid as it contains disallowed characters. Service names must adhere to the RFC 1123 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 and end with a lowercase alphanumeric character.",
adschwartz marked this conversation as resolved.
Show resolved Hide resolved
serviceName,
service.WordWrappedServiceNameRegex,
)
}

func replaceMagicStrings(
runtimeValueStore *runtime_value_store.RuntimeValueStore,
serviceName service.ServiceName,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ func (validator *StartosisValidator) Validate(ctx context.Context, instructions
serviceNamePortIdMapping)

isValidationFailure = isValidationFailure ||
validator.validateAnUpdateEnvironment(instructions, environment, starlarkRunResponseLineStream)
validator.validateAndUpdateEnvironment(instructions, environment, starlarkRunResponseLineStream)
logrus.Debug("Finished validating environment. Validating and downloading container images.")

isValidationFailure = isValidationFailure ||
Expand All @@ -77,7 +77,7 @@ func (validator *StartosisValidator) Validate(ctx context.Context, instructions
return starlarkRunResponseLineStream
}

func (validator *StartosisValidator) validateAnUpdateEnvironment(instructions []kurtosis_instruction.KurtosisInstruction, environment *startosis_validator.ValidatorEnvironment, starlarkRunResponseLineStream chan *kurtosis_core_rpc_api_bindings.StarlarkRunResponseLine) bool {
func (validator *StartosisValidator) validateAndUpdateEnvironment(instructions []kurtosis_instruction.KurtosisInstruction, environment *startosis_validator.ValidatorEnvironment, starlarkRunResponseLineStream chan *kurtosis_core_rpc_api_bindings.StarlarkRunResponseLine) bool {
isValidationFailure := false
for _, instruction := range instructions {
err := instruction.ValidateAndUpdateEnvironment(environment)
Expand Down
3 changes: 2 additions & 1 deletion docs/docs/cli-reference/service-add.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ kurtosis service add $THE_ENCLAVE_IDENTIFIER $THE_SERVICE_IDENTIFIER $CONTAINER_
```

where `$THE_ENCLAVE_IDENTIFIER` and the `$THE_SERVICE_IDENTIFIER` are [resource identifiers](../concepts-reference/resource-identifier.md) for the enclave and service, respectively.
Note, the service identifier needs to be formatted according to RFC 1123. Specifically, 1-63 lowercase alphanumeric characters with dashes and cannot start or end with dashes.

Much like `docker run`, this command has multiple options available to customize the service that's started:

Expand All @@ -22,4 +23,4 @@ To override the service's CMD, add a `--` after the image name and then pass in

```bash
kurtosis service add --entrypoint sh my-enclave test-service alpine -- -c "echo 'Hello world'"
```
```
3 changes: 2 additions & 1 deletion docs/docs/starlark-reference/plan.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ The `add_service` instruction adds a service to the Kurtosis enclave within whic
service = plan.add_service(
# The service name of the service being created.
# The service name is a reference to the service, which can be used in the future to refer to the service.
# Service names of active services are unique per enclave.
# Service names of active services are unique per enclave and needs to be formatted according to RFC 1123.
# Specifically, 1-63 lowercase alphanumeric characters with dashes and cannot start or end with dashes.
# MANDATORY
name = "example-datastore-server-1",

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ const (
executeNoDryRun = false
emptyArgs = "{}"

serviceName1 = "service_1"
serviceName2 = "service_2"
serviceName1 = "service-1"
serviceName2 = "service-2"
subnetwork1 = "subnetwork_1"
subnetwork2 = "subnetwork_2"
subnetwork3 = "subnetwork_3"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@ const (
executeNoDryRun = false
emptyArgs = "{}"

serviceId1 = "service_1"
serviceId2 = "service_2"
serviceId3 = "service_3"
serviceId1 = "service-1"
serviceId2 = "service-2"
serviceId3 = "service-3"
subnetwork1 = "subnetwork_1"

subnetworkInStarlarkScript = `DOCKER_GETTING_STARTED_IMAGE = "docker/getting-started:latest"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@ const (
executeNoDryRun = false
emptyArgs = "{}"

serviceId1 = "service_1"
serviceId2 = "service_2"
serviceId3 = "service_3"
serviceId1 = "service-1"
serviceId2 = "service-2"
serviceId3 = "service-3"
subnetwork1 = "subnetwork_1"
subnetwork2 = "subnetwork2"
subnetwork3 = "subnetwork3"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,5 +52,5 @@ func TestAddServiceWithInvalidServiceNameFailsValidation(t *testing.T) {

require.Nil(t, runResult.InterpretationError, "Unexpected interpretation error.")
require.NotEmpty(t, runResult.ValidationErrors, "Expected some validation errors")
require.Contains(t, runResult.ValidationErrors[0].ErrorMessage, fmt.Sprintf("Service name '%s' is invalid as it contains disallowed characters. Service names can only contain characters 'a-z', 'A-Z', '0-9', '-' & '_'", invalidServiceName))
require.Contains(t, runResult.ValidationErrors[0].ErrorMessage, fmt.Sprintf("Service name '%s' is invalid as it contains disallowed characters. Service names must adhere to the RFC 1123 standard, specifically implementing this regex and be 1-63 characters long: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?$. This means the service name must only contain lowercase alphanumeric characters or '-', and must start and end with a lowercase alphanumeric character.", invalidServiceName))
}