KEP-4427 : AllowRelaxedDNSSearchValidation (#127167)

* KEP-4427 : AllowRelaxedDNSSearchValidation

* Add e2e test with feature gate to test KEP-4427 RelaxedDNSSearchValidation

* Add more validatePodDNSConfig test cases

Also update Regex to match the case we want.

Thanks Tim and Antonio!
This commit is contained in:
Adrian Moisey 2024-09-12 10:41:19 +02:00 committed by GitHub
parent 9e59765585
commit 8e3adc4df6
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
11 changed files with 397 additions and 4 deletions

View File

@ -165,6 +165,12 @@ func SetDNSPolicy(policy api.DNSPolicy) Tweak {
}
}
func SetDNSConfig(config *api.PodDNSConfig) Tweak {
return func(pod *api.Pod) {
pod.Spec.DNSConfig = config
}
}
func SetRestartPolicy(policy api.RestartPolicy) Tweak {
return func(pod *api.Pod) {
pod.Spec.RestartPolicy = policy

View File

@ -391,6 +391,7 @@ func GetValidationOptionsFromPodSpecAndMeta(podSpec, oldPodSpec *api.PodSpec, po
// If old spec uses relaxed validation or enabled the RelaxedEnvironmentVariableValidation feature gate,
// we must allow it
opts.AllowRelaxedEnvironmentVariableValidation = useRelaxedEnvironmentVariableValidation(podSpec, oldPodSpec)
opts.AllowRelaxedDNSSearchValidation = useRelaxedDNSSearchValidation(oldPodSpec)
if oldPodSpec != nil {
// if old spec has status.hostIPs downwardAPI set, we must allow it
@ -448,6 +449,30 @@ func useRelaxedEnvironmentVariableValidation(podSpec, oldPodSpec *api.PodSpec) b
return false
}
func useRelaxedDNSSearchValidation(oldPodSpec *api.PodSpec) bool {
// Return true early if feature gate is enabled
if utilfeature.DefaultFeatureGate.Enabled(features.RelaxedDNSSearchValidation) {
return true
}
// Return false early if there is no DNSConfig or Searches.
if oldPodSpec == nil || oldPodSpec.DNSConfig == nil || oldPodSpec.DNSConfig.Searches == nil {
return false
}
return hasDotOrUnderscore(oldPodSpec.DNSConfig.Searches)
}
// Helper function to check if any domain is a dot or contains an underscore.
func hasDotOrUnderscore(searches []string) bool {
for _, domain := range searches {
if domain == "." || strings.Contains(domain, "_") {
return true
}
}
return false
}
func gatherPodEnvVarNames(podSpec *api.PodSpec) sets.Set[string] {
podEnvVarNames := sets.Set[string]{}

View File

@ -145,6 +145,15 @@ func ValidateQualifiedName(value string, fldPath *field.Path) field.ErrorList {
return allErrs
}
// ValidateDNS1123SubdomainWithUnderScore validates that a name is a proper DNS subdomain but allows for an underscore in the string
func ValidateDNS1123SubdomainWithUnderScore(value string, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}
for _, msg := range validation.IsDNS1123SubdomainWithUnderscore(value) {
allErrs = append(allErrs, field.Invalid(fldPath, value, msg))
}
return allErrs
}
// ValidateDNS1123Subdomain validates that a name is a proper DNS subdomain.
func ValidateDNS1123Subdomain(value string, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}
@ -3779,10 +3788,18 @@ func validatePodDNSConfig(dnsConfig *core.PodDNSConfig, dnsPolicy *core.DNSPolic
if len(strings.Join(dnsConfig.Searches, " ")) > MaxDNSSearchListChars {
allErrs = append(allErrs, field.Invalid(fldPath.Child("searches"), dnsConfig.Searches, fmt.Sprintf("must not have more than %v characters (including spaces) in the search list", MaxDNSSearchListChars)))
}
for i, search := range dnsConfig.Searches {
// it is fine to have a trailing dot
search = strings.TrimSuffix(search, ".")
allErrs = append(allErrs, ValidateDNS1123Subdomain(search, fldPath.Child("searches").Index(i))...)
if opts.AllowRelaxedDNSSearchValidation {
if search != "." {
search = strings.TrimSuffix(search, ".")
allErrs = append(allErrs, ValidateDNS1123SubdomainWithUnderScore(search, fldPath.Child("searches").Index(i))...)
}
} else {
search = strings.TrimSuffix(search, ".")
allErrs = append(allErrs, ValidateDNS1123Subdomain(search, fldPath.Child("searches").Index(i))...)
}
}
// Validate options.
for i, option := range dnsConfig.Options {
@ -4034,6 +4051,8 @@ type PodValidationOptions struct {
AllowRelaxedEnvironmentVariableValidation bool
// Allow the use of the ImageVolumeSource API.
AllowImageVolumeSource bool
// Allow the use of a relaxed DNS search
AllowRelaxedDNSSearchValidation bool
}
// validatePodMetadataAndSpec tests if required fields in the pod.metadata and pod.spec are set,

View File

@ -24193,6 +24193,172 @@ func TestValidateSleepAction(t *testing.T) {
}
}
// TODO: merge these test to TestValidatePodSpec after AllowRelaxedDNSSearchValidation feature graduates to Beta
func TestValidatePodDNSConfigWithRelaxedSearchDomain(t *testing.T) {
testCases := []struct {
name string
expectError bool
featureEnabled bool
dnsConfig *core.PodDNSConfig
}{
{
name: "beginswith underscore, contains underscore, featuregate enabled",
expectError: false,
featureEnabled: true,
dnsConfig: &core.PodDNSConfig{Searches: []string{"_sip._tcp.abc_d.example.com"}},
},
{
name: "contains underscore, featuregate enabled",
expectError: false,
featureEnabled: true,
dnsConfig: &core.PodDNSConfig{Searches: []string{"abc_d.example.com"}},
},
{
name: "is dot, featuregate enabled",
expectError: false,
featureEnabled: true,
dnsConfig: &core.PodDNSConfig{Searches: []string{"."}},
},
{
name: "two dots, featuregate enabled",
expectError: true,
featureEnabled: true,
dnsConfig: &core.PodDNSConfig{Searches: []string{".."}},
},
{
name: "underscore and dot, featuregate enabled",
expectError: true,
featureEnabled: true,
dnsConfig: &core.PodDNSConfig{Searches: []string{"_."}},
},
{
name: "dash and dot, featuregate enabled",
expectError: true,
featureEnabled: true,
dnsConfig: &core.PodDNSConfig{Searches: []string{"-."}},
},
{
name: "two underscore and dot, featuregate enabled",
expectError: true,
featureEnabled: true,
dnsConfig: &core.PodDNSConfig{Searches: []string{"__."}},
},
{
name: "dot and two underscore, featuregate enabled",
expectError: true,
featureEnabled: true,
dnsConfig: &core.PodDNSConfig{Searches: []string{".__"}},
},
{
name: "dot and underscore, featuregate enabled",
expectError: true,
featureEnabled: true,
dnsConfig: &core.PodDNSConfig{Searches: []string{"._"}},
},
{
name: "lot of underscores, featuregate enabled",
expectError: true,
featureEnabled: true,
dnsConfig: &core.PodDNSConfig{Searches: []string{"____________"}},
},
{
name: "a regular name, featuregate enabled",
expectError: false,
featureEnabled: true,
dnsConfig: &core.PodDNSConfig{Searches: []string{"example.com"}},
},
{
name: "unicode character, featuregate enabled",
expectError: true,
featureEnabled: true,
dnsConfig: &core.PodDNSConfig{Searches: []string{"☃.example.com"}},
},
{
name: "begins with underscore, contains underscore, featuregate disabled",
expectError: true,
featureEnabled: false,
dnsConfig: &core.PodDNSConfig{Searches: []string{"_sip._tcp.abc_d.example.com"}},
},
{
name: "contains underscore, featuregate disabled",
expectError: true,
featureEnabled: false,
dnsConfig: &core.PodDNSConfig{Searches: []string{"abc_d.example.com"}},
},
{
name: "is dot, featuregate disabled",
expectError: true,
featureEnabled: false,
dnsConfig: &core.PodDNSConfig{Searches: []string{"."}},
},
{
name: "two dots, featuregate disabled",
expectError: true,
featureEnabled: false,
dnsConfig: &core.PodDNSConfig{Searches: []string{".."}},
},
{
name: "underscore and dot, featuregate disabled",
expectError: true,
featureEnabled: false,
dnsConfig: &core.PodDNSConfig{Searches: []string{"_."}},
},
{
name: "dash and dot, featuregate disabled",
expectError: true,
featureEnabled: false,
dnsConfig: &core.PodDNSConfig{Searches: []string{"-."}},
},
{
name: "two underscore and dot, featuregate disabled",
expectError: true,
featureEnabled: false,
dnsConfig: &core.PodDNSConfig{Searches: []string{"__."}},
},
{
name: "dot and two underscore, featuregate disabled",
expectError: true,
featureEnabled: false,
dnsConfig: &core.PodDNSConfig{Searches: []string{".__"}},
},
{
name: "dot and underscore, featuregate disabled",
expectError: true,
featureEnabled: false,
dnsConfig: &core.PodDNSConfig{Searches: []string{"._"}},
},
{
name: "lot of underscores, featuregate disabled",
expectError: true,
featureEnabled: false,
dnsConfig: &core.PodDNSConfig{Searches: []string{"____________"}},
},
{
name: "a regular name, featuregate disabled",
expectError: false,
featureEnabled: false,
dnsConfig: &core.PodDNSConfig{Searches: []string{"example.com"}},
},
{
name: "unicode character, featuregate disabled",
expectError: true,
featureEnabled: false,
dnsConfig: &core.PodDNSConfig{Searches: []string{"☃.example.com"}},
},
}
for _, testCase := range testCases {
t.Run(testCase.name, func(t *testing.T) {
errs := validatePodDNSConfig(testCase.dnsConfig, nil, nil, PodValidationOptions{AllowRelaxedDNSSearchValidation: testCase.featureEnabled})
if testCase.expectError && len(errs) == 0 {
t.Errorf("Unexpected success")
}
if !testCase.expectError && len(errs) != 0 {
t.Errorf("Unexpected error(s): %v", errs)
}
})
}
}
// TODO: merge these test to TestValidatePodSpec after SupplementalGroupsPolicy feature graduates to Beta
func TestValidatePodSpecWithSupplementalGroupsPolicy(t *testing.T) {
fldPath := field.NewPath("spec")

View File

@ -618,6 +618,13 @@ const (
// Allow users to recover from volume expansion failure
RecoverVolumeExpansionFailure featuregate.Feature = "RecoverVolumeExpansionFailure"
// owner: @adrianmoisey
// kep: https://kep.k8s.io/4427
// alpha: v1.32
//
// Relaxed DNS search string validation.
RelaxedDNSSearchValidation featuregate.Feature = "RelaxedDNSSearchValidation"
// owner: @HirazawaUi
// kep: https://kep.k8s.io/4369
// alpha: v1.30

View File

@ -292,7 +292,9 @@ var defaultVersionedKubernetesFeatureGates = map[featuregate.Feature]featuregate
{Version: version.MustParse("1.31"), Default: false, PreRelease: featuregate.Alpha},
{Version: version.MustParse("1.32"), Default: true, PreRelease: featuregate.Beta},
},
RelaxedDNSSearchValidation: {
{Version: version.MustParse("1.32"), Default: false, PreRelease: featuregate.Alpha},
},
RelaxedEnvironmentVariableValidation: {
{Version: version.MustParse("1.30"), Default: false, PreRelease: featuregate.Alpha},
},

View File

@ -175,6 +175,8 @@ func IsValidLabelValue(value string) []string {
}
const dns1123LabelFmt string = "[a-z0-9]([-a-z0-9]*[a-z0-9])?"
const dns1123LabelFmtWithUnderscore string = "_?[a-z0-9]([-_a-z0-9]*[a-z0-9])?"
const dns1123LabelErrMsg string = "a lowercase RFC 1123 label must consist of lower case alphanumeric characters or '-', and must start and end with an alphanumeric character"
// DNS1123LabelMaxLength is a label's max length in DNS (RFC 1123)
@ -204,10 +206,14 @@ func IsDNS1123Label(value string) []string {
const dns1123SubdomainFmt string = dns1123LabelFmt + "(\\." + dns1123LabelFmt + ")*"
const dns1123SubdomainErrorMsg string = "a lowercase RFC 1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character"
const dns1123SubdomainFmtWithUnderscore string = dns1123LabelFmtWithUnderscore + "(\\." + dns1123LabelFmtWithUnderscore + ")*"
const dns1123SubdomainErrorMsgFG string = "a lowercase RFC 1123 subdomain must consist of lower case alphanumeric characters, '_', '-' or '.', and must start and end with an alphanumeric character"
// DNS1123SubdomainMaxLength is a subdomain's max length in DNS (RFC 1123)
const DNS1123SubdomainMaxLength int = 253
var dns1123SubdomainRegexp = regexp.MustCompile("^" + dns1123SubdomainFmt + "$")
var dns1123SubdomainRegexpWithUnderscore = regexp.MustCompile("^" + dns1123SubdomainFmtWithUnderscore + "$")
// IsDNS1123Subdomain tests for a string that conforms to the definition of a
// subdomain in DNS (RFC 1123).
@ -222,6 +228,19 @@ func IsDNS1123Subdomain(value string) []string {
return errs
}
// IsDNS1123SubdomainWithUnderscore tests for a string that conforms to the definition of a
// subdomain in DNS (RFC 1123), but allows the use of an underscore in the string
func IsDNS1123SubdomainWithUnderscore(value string) []string {
var errs []string
if len(value) > DNS1123SubdomainMaxLength {
errs = append(errs, MaxLenError(DNS1123SubdomainMaxLength))
}
if !dns1123SubdomainRegexpWithUnderscore.MatchString(value) {
errs = append(errs, RegexError(dns1123SubdomainErrorMsgFG, dns1123SubdomainFmt, "example.com"))
}
return errs
}
const dns1035LabelFmt string = "[a-z]([-a-z0-9]*[a-z0-9])?"
const dns1035LabelErrMsg string = "a DNS-1035 label must consist of lower case alphanumeric characters or '-', start with an alphabetic character, and end with an alphanumeric character"

View File

@ -283,6 +283,10 @@ var (
// and whether the pod can consume configmap/secret that key starts with a number.
RelaxedEnvironmentVariableValidation = framework.WithFeature(framework.ValidFeatures.Add("RelaxedEnvironmentVariableValidation"))
// Owner: sig-network
// Marks tests of KEP-4427 that require the `RelaxedDNSSearchValidation` feature gate
RelaxedDNSSearchValidation = framework.WithFeature(framework.ValidFeatures.Add("RelaxedDNSSearchValidation"))
// TODO: document the feature (owning SIG, when to use this feature for a test)
Recreate = framework.WithFeature(framework.ValidFeatures.Add("Recreate"))

View File

@ -25,6 +25,7 @@ import (
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/wait"
"k8s.io/kubernetes/test/e2e/feature"
"k8s.io/kubernetes/test/e2e/framework"
e2enode "k8s.io/kubernetes/test/e2e/framework/node"
e2epod "k8s.io/kubernetes/test/e2e/framework/pod"
@ -597,7 +598,39 @@ var _ = common.SIGDescribe("DNS", func() {
}
validateDNSResults(ctx, f, pod, append(wheezyFileNames, jessieFileNames...))
})
})
// TODO replace WithLabel by framework.WithFeatureGate(features.RelaxedDNSSearchValidation) once https://github.com/kubernetes/kubernetes/pull/126977 is solved
var _ = common.SIGDescribe("DNS", feature.RelaxedDNSSearchValidation, framework.WithLabel("Feature:Alpha"), func() {
f := framework.NewDefaultFramework("dns")
f.NamespacePodSecurityLevel = admissionapi.LevelBaseline
ginkgo.It("should work with a search path containing an underscore and a search path with a single dot", func(ctx context.Context) {
// All the names we need to be able to resolve.
namesToResolve := []string{
"kubernetes.default",
"kubernetes.default.svc",
}
hostFQDN := fmt.Sprintf("%s.%s.%s.svc.%s", dnsTestPodHostName, dnsTestServiceName, f.Namespace.Name, framework.TestContext.ClusterDNSDomain)
hostEntries := []string{hostFQDN, dnsTestPodHostName}
// TODO: Validate both IPv4 and IPv6 families for dual-stack
wheezyProbeCmd, wheezyFileNames := createProbeCommand(namesToResolve, hostEntries, "", "wheezy", f.Namespace.Name, framework.TestContext.ClusterDNSDomain, framework.TestContext.ClusterIsIPv6())
jessieProbeCmd, jessieFileNames := createProbeCommand(namesToResolve, hostEntries, "", "jessie", f.Namespace.Name, framework.TestContext.ClusterDNSDomain, framework.TestContext.ClusterIsIPv6())
ginkgo.By("Running these commands on wheezy: " + wheezyProbeCmd + "\n")
ginkgo.By("Running these commands on jessie: " + jessieProbeCmd + "\n")
ginkgo.By("Creating a pod with expanded DNS configuration to probe DNS")
testSearchPaths := []string{
".",
"_sip._tcp.abc_d.example.com",
}
pod := createDNSPod(f.Namespace.Name, wheezyProbeCmd, jessieProbeCmd, dnsTestPodHostName, dnsTestServiceName)
pod.Spec.DNSPolicy = v1.DNSClusterFirst
pod.Spec.DNSConfig = &v1.PodDNSConfig{
Searches: testSearchPaths,
}
validateDNSResults(ctx, f, pod, append(wheezyFileNames, jessieFileNames...))
})
})
var _ = common.SIGDescribe("DNS HostNetwork", func() {

View File

@ -694,6 +694,12 @@
lockToDefault: false
preRelease: Alpha
version: "1.23"
- name: RelaxedDNSSearchValidation
versionedSpecs:
- default: false
lockToDefault: false
preRelease: Alpha
version: "1.32"
- name: RelaxedEnvironmentVariableValidation
versionedSpecs:
- default: false

View File

@ -25,9 +25,12 @@ import (
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
utilfeature "k8s.io/apiserver/pkg/util/feature"
clientset "k8s.io/client-go/kubernetes"
typedv1 "k8s.io/client-go/kubernetes/typed/core/v1"
featuregatetesting "k8s.io/component-base/featuregate/testing"
kubeapiservertesting "k8s.io/kubernetes/cmd/kube-apiserver/app/testing"
"k8s.io/kubernetes/pkg/features"
"k8s.io/kubernetes/test/integration"
"k8s.io/kubernetes/test/integration/framework"
)
@ -872,3 +875,106 @@ func TestMutablePodSchedulingDirectives(t *testing.T) {
integration.DeletePodOrErrorf(t, client, ns.Name, tc.update.Name)
}
}
// Test disabling of RelaxedDNSSearchValidation after a Pod has been created
func TestRelaxedDNSSearchValidation(t *testing.T) {
// Disable ServiceAccount admission plugin as we don't have serviceaccount controller running.
server := kubeapiservertesting.StartTestServerOrDie(t,
&kubeapiservertesting.TestServerInstanceOptions{BinaryVersion: "1.32"},
framework.DefaultTestServerFlags(), framework.SharedEtcd())
defer server.TearDownFn()
client := clientset.NewForConfigOrDie(server.ClientConfig)
ns := framework.CreateNamespaceOrDie(client, "pod-update-dns-search", t)
defer framework.DeleteNamespaceOrDie(client, ns, t)
testPod := func(name string) *v1.Pod {
return &v1.Pod{
ObjectMeta: metav1.ObjectMeta{
Name: name,
},
Spec: v1.PodSpec{
Containers: []v1.Container{
{
Name: "fake-name",
Image: "fakeimage",
},
},
},
}
}
cases := []struct {
name string
original *v1.PodDNSConfig
valid bool
featureGateEnabled bool
update bool
}{
{
name: "new pod with underscore - feature gate enabled",
original: &v1.PodDNSConfig{Searches: []string{"_sip._tcp.abc_d.example.com"}},
valid: true,
featureGateEnabled: true,
},
{
name: "new pod with dot - feature gate enabled",
original: &v1.PodDNSConfig{Searches: []string{"."}},
valid: true,
featureGateEnabled: true,
},
{
name: "new pod without underscore - feature gate enabled",
original: &v1.PodDNSConfig{Searches: []string{"example.com"}},
valid: true,
featureGateEnabled: true,
},
{
name: "new pod with underscore - feature gate disabled",
original: &v1.PodDNSConfig{Searches: []string{"_sip._tcp.abc_d.example.com"}},
valid: false,
featureGateEnabled: false,
},
{
name: "new pod with dot - feature gate disabled",
original: &v1.PodDNSConfig{Searches: []string{"."}},
valid: false,
featureGateEnabled: false,
},
{
name: "new pod without underscore - feature gate disabled",
original: &v1.PodDNSConfig{Searches: []string{"example.com"}},
valid: true,
featureGateEnabled: false,
},
}
for _, tc := range cases {
featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.RelaxedDNSSearchValidation, tc.featureGateEnabled)
pod := testPod("dns")
pod.Spec.DNSConfig = tc.original
_, err := client.CoreV1().Pods(ns.Name).Create(context.TODO(), pod, metav1.CreateOptions{})
if tc.valid && err != nil {
t.Errorf("%v: %v", tc.name, err)
} else if !tc.valid && err == nil {
t.Errorf("%v: unexpected allowed update to ephemeral containers", tc.name)
}
// Disable gate and perform update
featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.RelaxedDNSSearchValidation, false)
pod.ObjectMeta.Labels = map[string]string{"label": "value"}
_, err = client.CoreV1().Pods(ns.Name).Update(context.TODO(), pod, metav1.UpdateOptions{})
if tc.valid && err != nil {
t.Errorf("%v: failed to update ephemeral containers: %v", tc.name, err)
} else if !tc.valid && err == nil {
t.Errorf("%v: unexpected allowed update to ephemeral containers", tc.name)
}
if tc.valid {
integration.DeletePodOrErrorf(t, client, ns.Name, pod.Name)
}
}
}