Skip to content

Commit

Permalink
fix: make service labels more restrictive (#929)
Browse files Browse the repository at this point in the history
## Description:
Barnabas at Eth tried running the latest package changes and ran into
errors with the naming scheme on k8s.

## Is this change user facing?
YES

Closes #928
  • Loading branch information
h4ck3rk3y committed Jul 18, 2023
1 parent 365f5cf commit a8fb599
Show file tree
Hide file tree
Showing 8 changed files with 22 additions and 13 deletions.
Expand Up @@ -34,12 +34,13 @@ func (key *KubernetesObjectName) GetString() string {
}

// https://github.com/kubernetes/design-proposals-archive/blob/main/architecture/identifiers.md
// In Kubernetes, to create an object you must specify a 'name' that is a DNS_LABEL, following rfc1123
// Most object names are valid rfc1123 DNS_SUBDOMAIN, but some are not. All Object names are valid DNS_LABEL
// In Kubernetes, to create an object you must specify a 'name' that is a DNS_LABEL, following rfc1035
// Most object names are valid rfc1035 DNS_SUBDOMAIN, but some are not. All Object names are valid DNS_LABEL
// We chose DNS_LABEL, to ensure that our object names will always be valid in Kubernetes
// We used to use rfc1123 but we are using rfc1035 as its more restrictive adn a requirement for ServiceNames
// https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names
func validateKubernetesObjectName(str string) error {
validationErrs := validation.IsDNS1123Label(str)
validationErrs := validation.IsDNS1035Label(str)
if len(validationErrs) > 0 {
errString := strings.Join(validationErrs, "\n\n")
return stacktrace.NewError("Expected object name string '%v' to be a valid DNS_LABEL, instead it failed validation:\n%+v", str, errString)
Expand Down
Expand Up @@ -8,14 +8,16 @@ import (
)

const (
// ServiceNameRegex implements RFC-1123 for naming services, namely:
// ServiceNameRegex implements RFC-1035 for naming services, namely:
// * contain at most 63 characters
// * contain only lowercase alphanumeric characters or '-'
// * start with an alphanumeric character
// * start with an alphabetic character
// * end with an alphanumeric character
// The adoption of RFC-1123 is to maintain compatability with current Kubernetes service and pod naming standards:
// 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
ServiceNameRegex = "[a-z0-9]([-a-z0-9]{0,61}[a-z0-9])?"
// https://kubernetes.io/docs/concepts/services-networking/service/
ServiceNameRegex = "[a-z]([-a-z0-9]{0,61}[a-z0-9])?"
WordWrappedServiceNameRegex = "^" + ServiceNameRegex + "$"
serviceNameMaxLength = 63
)
Expand Down
Expand Up @@ -11,6 +11,10 @@ func TestValidServiceName(t *testing.T) {
require.True(t, IsServiceNameValid(ServiceName("abcdefabcdefabcdefabcdefabcdefabcdefabcdefabcdefabcdefabcdef123")))
}

func TestInvalidServiceName(t *testing.T) {
require.False(t, IsServiceNameValid(ServiceName("1-geth-lighthouse")))
}

func TestServiceNameWithSpecialChars(t *testing.T) {
require.True(t, IsServiceNameValid(ServiceName("a-b")))
require.False(t, IsServiceNameValid(ServiceName("-bc")))
Expand Down
Expand Up @@ -93,7 +93,7 @@ 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.",
"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,
)
Expand Down
@@ -1,7 +1,6 @@
package magic_string_helper

import (
"github.com/kurtosis-tech/kurtosis/container-engine-lib/lib/backend_interface/objects/service"
"github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/runtime_value_store"
"github.com/kurtosis-tech/stacktrace"
"go.starlark.net/starlark"
Expand All @@ -20,8 +19,9 @@ const (
runtimeValueSubgroupName = "runtime_value"
runtimeValueFieldSubgroupName = "runtime_value_field"
runtimeValueKeyRegexp = "[a-zA-Z0-9-_\\.]+"
uuidFormat = "[a-f0-9]{32}"

runtimeValueReplacementRegex = "(?P<" + allSubgroupName + ">\\{\\{" + kurtosisNamespace + ":(?P<" + runtimeValueSubgroupName + ">" + service.ServiceNameRegex + ")" + ":(?P<" + runtimeValueFieldSubgroupName + ">" + runtimeValueKeyRegexp + ")\\.runtime_value\\}\\})"
runtimeValueReplacementRegex = "(?P<" + allSubgroupName + ">\\{\\{" + kurtosisNamespace + ":(?P<" + runtimeValueSubgroupName + ">" + uuidFormat + ")" + ":(?P<" + runtimeValueFieldSubgroupName + ">" + runtimeValueKeyRegexp + ")\\.runtime_value\\}\\})"
RuntimeValueReplacementPlaceholderFormat = "{{" + kurtosisNamespace + ":%v:%v.runtime_value}}"

subExpNotFound = -1
Expand Down
3 changes: 2 additions & 1 deletion docs/docs/cli-reference/service-add.md
Expand Up @@ -11,7 +11,8 @@ 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.
Note, the service identifier needs to be formatted according to RFC 1035. Specifically, 1-63 lowercase alphanumeric characters with dashes and cannot start or end with dashes. Also service names
have to start with a lowercase alphabet.

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

Expand Down
3 changes: 2 additions & 1 deletion docs/docs/starlark-reference/plan.md
Expand Up @@ -21,8 +21,9 @@ 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 and needs to be formatted according to RFC 1123.
# Service names of active services are unique per enclave and needs to be formatted according to RFC 1035.
# Specifically, 1-63 lowercase alphanumeric characters with dashes and cannot start or end with dashes.
# Also service names have to start with a lowercase alphabet.
# MANDATORY
name = "example-datastore-server-1",

Expand Down
Expand Up @@ -34,5 +34,5 @@ func (suite *StartosisAddServiceTestSuite) TestAddServiceWithInvalidServiceNameF

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 must adhere to the RFC 1123 standard, specifically implementing this regex and be 1-63 characters long: ^[a-z0-9]([-a-z0-9]{0,61}[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))
require.Contains(t, runResult.ValidationErrors[0].ErrorMessage, 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: ^[a-z]([-a-z0-9]{0,61}[a-z0-9])?$. 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.", invalidServiceName))
}

0 comments on commit a8fb599

Please sign in to comment.