Identify and block malformed NO_PROXY values (#962)

Author: HarrisonWAffel

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

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

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