diff --git a/docs.md b/docs.md index 22a1c4215..b54fa1d20 100644 --- a/docs.md +++ b/docs.md @@ -558,6 +558,11 @@ The annotation `field.cattle.io/creatorId` must be set to the Username of the Us If `field.cattle.io/no-creator-rbac` annotation is set, `field.cattle.io/creatorId` cannot be set. +##### NO_PROXY value + +Prevent the creation of new objects with an env var (under `spec.agentEnvVars`) with a name of `NO_PROXY` if its value contains one or more spaces. This ensures that the provided value adheres to +the format expected by Go, and helps to prevent subtle issues elsewhere when writing scripts which utilize `NO_PROXY`. + ##### Data Directories Prevent the creation of new objects with an env var (under `spec.agentEnvVars`) with a name of `CATTLE_AGENT_VAR_DIR`. @@ -619,6 +624,13 @@ Both `minAvailable` and `maxUnavailable` must be a string which represents a non ^([0-9]|[1-9][0-9]|100)%$ ``` +##### NO_PROXY value + +Prevent the update of objects with an env var (under `spec.agentEnvVars`) with a name of `NO_PROXY` if its value contains one or more spaces. This ensures that the provided value adheres to +the format expected by Go, and helps to prevent subtle issues elsewhere when writing scripts which utilize `NO_PROXY`. + +The only exception to this check is if the existing cluster already has a `NO_PROXY` variable which includes spaces in its value. In this case, update operations are permitted. If `NO_PROXY` is later updated to value which does not contain spaces, this exception will no longer occur. + ### Mutation Checks #### On Create diff --git a/pkg/resources/provisioning.cattle.io/v1/cluster/Cluster.md b/pkg/resources/provisioning.cattle.io/v1/cluster/Cluster.md index f741d61d1..07753c3f0 100644 --- a/pkg/resources/provisioning.cattle.io/v1/cluster/Cluster.md +++ b/pkg/resources/provisioning.cattle.io/v1/cluster/Cluster.md @@ -8,6 +8,11 @@ The annotation `field.cattle.io/creatorId` must be set to the Username of the Us If `field.cattle.io/no-creator-rbac` annotation is set, `field.cattle.io/creatorId` cannot be set. +#### NO_PROXY value + +Prevent the creation of new objects with an env var (under `spec.agentEnvVars`) with a name of `NO_PROXY` if its value contains one or more spaces. This ensures that the provided value adheres to +the format expected by Go, and helps to prevent subtle issues elsewhere when writing scripts which utilize `NO_PROXY`. + #### Data Directories Prevent the creation of new objects with an env var (under `spec.agentEnvVars`) with a name of `CATTLE_AGENT_VAR_DIR`. @@ -69,6 +74,13 @@ Both `minAvailable` and `maxUnavailable` must be a string which represents a non ^([0-9]|[1-9][0-9]|100)%$ ``` +#### NO_PROXY value + +Prevent the update of objects with an env var (under `spec.agentEnvVars`) with a name of `NO_PROXY` if its value contains one or more spaces. This ensures that the provided value adheres to +the format expected by Go, and helps to prevent subtle issues elsewhere when writing scripts which utilize `NO_PROXY`. + +The only exception to this check is if the existing cluster already has a `NO_PROXY` variable which includes spaces in its value. In this case, update operations are permitted. If `NO_PROXY` is later updated to value which does not contain spaces, this exception will no longer occur. + ## Mutation Checks ### On Create diff --git a/pkg/resources/provisioning.cattle.io/v1/cluster/validator.go b/pkg/resources/provisioning.cattle.io/v1/cluster/validator.go index df2acc535..f4774e30c 100644 --- a/pkg/resources/provisioning.cattle.io/v1/cluster/validator.go +++ b/pkg/resources/provisioning.cattle.io/v1/cluster/validator.go @@ -139,6 +139,10 @@ func (p *provisioningAdmitter) Admit(request *admission.Request) (*admissionv1.A return response, err } + if response = validateHTTPNoProxyVariable(request, oldCluster, cluster); !response.Allowed { + return response, nil + } + if response = p.validateDataDirectories(request, oldCluster, cluster); !response.Allowed { return response, err } @@ -408,6 +412,29 @@ func (p *provisioningAdmitter) validateClusterName(request *admission.Request, r return nil } +func validateHTTPNoProxyVariable(request *admission.Request, oldCluster, newCluster *v1.Cluster) *admissionv1.AdmissionResponse { + switch request.Operation { + case admissionv1.Create: + proxyValue := retrieveNoProxy(newCluster) + if proxyValue != "" && strings.Contains(proxyValue, " ") { + return admission.ResponseBadRequest("Malformed NO_PROXY environment variable value format: detected whitespace in value. Value must be a comma-delimited string with no spaces containing one or more IP address prefixes (1.2.3.4, 1.2.3.4:80), IP address prefixes in CIDR notation (1.2.3.4/8), domain names, or special DNS labels (*)") + } + case admissionv1.Update: + // Block updating existing clusters with a malformed NO_PROXY, but + // don't block existing clusters which already have a malformed value. + oldProxyValue := retrieveNoProxy(oldCluster) + newProxyValue := retrieveNoProxy(newCluster) + + alreadyHadSpaces := oldProxyValue != "" && strings.Contains(oldProxyValue, " ") + currentlyContainsSpaces := newProxyValue != "" && strings.Contains(newProxyValue, " ") + if !alreadyHadSpaces && currentlyContainsSpaces { + return admission.ResponseBadRequest("Malformed NO_PROXY environment variable value format: detected whitespace in value. Value must be a comma-delimited string with no spaces containing one or more IP address prefixes (1.2.3.4, 1.2.3.4:80), IP address prefixes in CIDR notation (1.2.3.4/8), domain names, or special DNS labels (*)") + } + } + + return admission.ResponseAllowed() +} + func (p *provisioningAdmitter) validateMachinePoolNames(request *admission.Request, response *admissionv1.AdmissionResponse, cluster *v1.Cluster) error { if request.Operation != admissionv1.Create { return nil @@ -863,3 +890,15 @@ func isValidName(clusterName, clusterNamespace string, clusterExists bool) bool // RFC-1123 will fail to deploy required fleet components and should not be accepted. return len(clusterName) <= 63 && fleetNameRegex.MatchString(clusterName) } + +func retrieveNoProxy(cluster *v1.Cluster) string { + if cluster == nil { + return "" + } + for _, envVar := range cluster.Spec.AgentEnvVars { + if strings.ToLower(envVar.Name) == "no_proxy" { + return envVar.Value + } + } + return "" +} diff --git a/pkg/resources/provisioning.cattle.io/v1/cluster/validator_test.go b/pkg/resources/provisioning.cattle.io/v1/cluster/validator_test.go index a8493cd28..08fc59090 100644 --- a/pkg/resources/provisioning.cattle.io/v1/cluster/validator_test.go +++ b/pkg/resources/provisioning.cattle.io/v1/cluster/validator_test.go @@ -147,6 +147,313 @@ func Test_isValidName(t *testing.T) { } } +func TestValidNoProxy(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + oldCluster *v1.Cluster + newCluster *v1.Cluster + request *admission.Request + expected bool + }{ + { + name: "valid cluster create operation", + request: &admission.Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ + Operation: admissionv1.Create, + }, + }, + newCluster: &v1.Cluster{ + Spec: v1.ClusterSpec{ + AgentEnvVars: []rkev1.EnvVar{ + { + Name: "NO_PROXY", + Value: "valid,value", + }, + }, + }, + }, + expected: true, + }, + { + name: "valid cluster create operation lowercase", + request: &admission.Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ + Operation: admissionv1.Create, + }, + }, + newCluster: &v1.Cluster{ + Spec: v1.ClusterSpec{ + AgentEnvVars: []rkev1.EnvVar{ + { + Name: "no_proxy", + Value: "valid,value", + }, + }, + }, + }, + expected: true, + }, + { + name: "invalid cluster create operation", + request: &admission.Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ + Operation: admissionv1.Create, + }, + }, + newCluster: &v1.Cluster{ + Spec: v1.ClusterSpec{ + AgentEnvVars: []rkev1.EnvVar{ + { + Name: "NO_PROXY", + Value: "something bad", + }, + }, + }, + }, + expected: false, + }, + { + name: "invalid cluster create operation lowercase", + request: &admission.Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ + Operation: admissionv1.Create, + }, + }, + newCluster: &v1.Cluster{ + Spec: v1.ClusterSpec{ + AgentEnvVars: []rkev1.EnvVar{ + { + Name: "no_proxy", + Value: "something bad", + }, + }, + }, + }, + expected: false, + }, + { + name: "valid cluster update operation", + request: &admission.Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ + Operation: admissionv1.Update, + }, + }, + oldCluster: &v1.Cluster{ + Spec: v1.ClusterSpec{ + AgentEnvVars: []rkev1.EnvVar{}, + }, + }, + newCluster: &v1.Cluster{ + Spec: v1.ClusterSpec{ + AgentEnvVars: []rkev1.EnvVar{ + { + Name: "NO_PROXY", + Value: "valid,value", + }, + }, + }, + }, + expected: true, + }, + { + name: "valid cluster update operation lowercase", + request: &admission.Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ + Operation: admissionv1.Update, + }, + }, + oldCluster: &v1.Cluster{ + Spec: v1.ClusterSpec{ + AgentEnvVars: []rkev1.EnvVar{}, + }, + }, + newCluster: &v1.Cluster{ + Spec: v1.ClusterSpec{ + AgentEnvVars: []rkev1.EnvVar{ + { + Name: "no_proxy", + Value: "valid,value", + }, + }, + }, + }, + expected: true, + }, + { + name: "valid cluster update operation", + request: &admission.Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ + Operation: admissionv1.Update, + }, + }, + oldCluster: &v1.Cluster{ + Spec: v1.ClusterSpec{ + AgentEnvVars: []rkev1.EnvVar{ + { + Name: "NO_PROXY", + Value: "previous,value", + }, + }, + }, + }, + newCluster: &v1.Cluster{ + Spec: v1.ClusterSpec{ + AgentEnvVars: []rkev1.EnvVar{ + { + Name: "NO_PROXY", + Value: "valid,value", + }, + }, + }, + }, + expected: true, + }, + { + name: "valid malformed cluster update operation", + request: &admission.Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ + Operation: admissionv1.Update, + }, + }, + oldCluster: &v1.Cluster{ + Spec: v1.ClusterSpec{ + AgentEnvVars: []rkev1.EnvVar{ + { + Name: "NO_PROXY", + Value: "previous, bad , value", + }, + }, + }, + }, + newCluster: &v1.Cluster{ + Spec: v1.ClusterSpec{ + AgentEnvVars: []rkev1.EnvVar{ + { + Name: "NO_PROXY", + Value: "new, bad, value", + }, + }, + }, + }, + expected: true, + }, + { + name: "invalid cluster update operation", + request: &admission.Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ + Operation: admissionv1.Update, + }, + }, + oldCluster: &v1.Cluster{ + Spec: v1.ClusterSpec{ + AgentEnvVars: []rkev1.EnvVar{}, + }, + }, + newCluster: &v1.Cluster{ + Spec: v1.ClusterSpec{ + AgentEnvVars: []rkev1.EnvVar{ + { + Name: "NO_PROXY", + Value: "new, bad, value", + }, + }, + }, + }, + expected: false, + }, + { + name: "invalid cluster update operation lowercase", + request: &admission.Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ + Operation: admissionv1.Update, + }, + }, + oldCluster: &v1.Cluster{ + Spec: v1.ClusterSpec{ + AgentEnvVars: []rkev1.EnvVar{}, + }, + }, + newCluster: &v1.Cluster{ + Spec: v1.ClusterSpec{ + AgentEnvVars: []rkev1.EnvVar{ + { + Name: "no_proxy", + Value: "new, bad, value", + }, + }, + }, + }, + expected: false, + }, + { + name: "invalid cluster update operation", + request: &admission.Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ + Operation: admissionv1.Update, + }, + }, + oldCluster: &v1.Cluster{ + Spec: v1.ClusterSpec{ + AgentEnvVars: []rkev1.EnvVar{ + { + Name: "NO_PROXY", + Value: "a,previous,value", + }, + }, + }, + }, + newCluster: &v1.Cluster{ + Spec: v1.ClusterSpec{ + AgentEnvVars: []rkev1.EnvVar{ + { + Name: "NO_PROXY", + Value: "new, bad, value", + }, + }, + }, + }, + expected: false, + }, + } + + for _, tst := range tests { + tst := tst + t.Run(tst.name, func(t *testing.T) { + resp := validateHTTPNoProxyVariable(tst.request, tst.oldCluster, tst.newCluster) + + var oldValue, newValue string + if tst.newCluster != nil && len(tst.newCluster.Spec.AgentEnvVars) > 0 { + newValue = tst.newCluster.Spec.AgentEnvVars[0].Value + } + if tst.oldCluster != nil && len(tst.oldCluster.Spec.AgentEnvVars) > 0 { + oldValue = tst.oldCluster.Spec.AgentEnvVars[0].Value + } + + if (resp.Result == nil || resp.Result.Status != failureStatus) && !tst.expected { + if oldValue == "" && newValue != "" { + t.Logf("Expected error when providing NO_PROXY value of '%s'", newValue) + } + if oldValue != "" && newValue != "" { + t.Logf("Expected error when updating from old value of '%s' to new value of '%s'", oldValue, newValue) + } + t.Fail() + } + + if (resp.Result != nil && resp.Result.Status == failureStatus) && tst.expected { + if oldValue == "" && newValue != "" { + t.Logf("Encountered unexpected error when providing NO_PROXY value of '%s'", newValue) + } + if oldValue != "" && newValue != "" { + t.Logf("Encountered unexpected error when updating from old value of '%s' to new value of '%s'", oldValue, newValue) + } + t.Fail() + } + }) + } +} + func TestValidateMachinePoolName(t *testing.T) { t.Parallel()