Check if aws_vpc_cidr is empty before adding to no_proxy list (#3044)

* Check if aws_vpc_cidr is empty before adding to TKG_NO_PROXY list
* Test removing sorted
* Forbid user set * inside TKG_NO_PROXY
* Remove space in TKG_NO_PROXY

pkg/v1/providers/kapp-controller-values/helpers.star

@@ -3,6 +3,7 @@
 #! Any changes affecting this helper functions need to be done at both the places
 
 load("@ytt:data", "data")
+load("@ytt:regexp", "regexp")
 
 def tkg_image_repo_customized():
     return data.values.TKG_CUSTOM_IMAGE_REPOSITORY != ""
@@ -24,14 +25,19 @@ def get_no_proxy():
   if data.values.TKG_HTTP_PROXY != "":
     full_no_proxy_list = []
     if data.values.TKG_NO_PROXY != "":
-      full_no_proxy_list = data.values.TKG_NO_PROXY.split(",")
+      # trim space in the no_proxy list
+      full_no_proxy_list = regexp.replace(" ", data.values.TKG_NO_PROXY, "").split(",")
     end
     if data.values.PROVIDER_TYPE == "aws":
-      full_no_proxy_list.append(data.values.AWS_VPC_CIDR)
+      if data.values.AWS_VPC_CIDR != "":
+        full_no_proxy_list.append(data.values.AWS_VPC_CIDR)
+      end
       full_no_proxy_list.append("169.254.0.0/16")
     end
     if data.values.PROVIDER_TYPE == "azure":
-      full_no_proxy_list.append(data.values.AZURE_VNET_CIDR)
+      if data.values.AZURE_VNET_CIDR != "":
+        full_no_proxy_list.append(data.values.AZURE_VNET_CIDR)
+      end
       full_no_proxy_list.append("169.254.0.0/16")
       full_no_proxy_list.append("168.63.129.16")
     end

pkg/v1/providers/tests/clustergen/param_models/cluster_optional.model

@@ -93,7 +93,7 @@ VSPHERE_CLONE_MODE: "fullClone","linkedClone"
 # http proxy (too many "NA" leads to overwhelmingly negative cases)
 TKG_HTTP_PROXY: "http://10.0.200.100", "http://[fc00:f853:ccd:e793::1]:3128"
 TKG_HTTPS_PROXY: "NA", "http://10.0.200.100", "http://[fc00:f853:ccd:e793::1]:3128"
-TKG_NO_PROXY: "NA", "10.0.200.1/24", "10.184.90.80"
+TKG_NO_PROXY: "NA", "10.0.200.1/24", "10.184.90.80<comma>        10.0.0.1/24"
 TKG_PROXY_CA_CERT: "NA", "LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tClBST1hZIENBIENFUlQ="
 TKG_HTTP_PROXY_ENABLED: "true"
 

pkg/v1/providers/ytt/03_customizations/http_proxy.yaml

@@ -15,7 +15,7 @@
 #@ end
 
 #@ def exportEnvvar(key, val):
-#@ return "export {}={}".format(key, val)
+#@ return "export {}='{}'".format(key, val)
 #@ end
 
 #@ if data.values.TKG_HTTP_PROXY != "":

pkg/v1/providers/ytt/09_miscellaneous/08_store_data_values.yaml

@@ -2,6 +2,7 @@
 #@ load("@ytt:yaml", "yaml")
 #@ load("@ytt:struct", "struct")
 #@ load("/lib/config_variable_association.star", "config_variable_association")
+#@ load("@ytt:regexp", "regexp")
 
 #@ list_skip_variable_from_storing = [
 #@  "VSPHERE_PASSWORD",
@@ -24,8 +25,8 @@
 #@ if configVariable not in list_skip_variable_from_storing and data.values.PROVIDER_TYPE in kvs[configVariable]:
 #@ value = data.values[configVariable]
 #@ if configVariable == "TKG_NO_PROXY":
-#@   no_proxy_list = data.values.TKG_NO_PROXY.split(",")
-#@   value = ",".join(sorted(list(set(no_proxy_list))))
+#@   no_proxy_list = regexp.replace(" ", data.values.TKG_NO_PROXY, "").split(",")
+#@   value = ",".join(list(set(no_proxy_list)))
 #@ end
 
 #@yaml/text-templated-strings

pkg/v1/providers/ytt/lib/helpers.star

@@ -290,14 +290,19 @@ def get_no_proxy():
   if data.values.TKG_HTTP_PROXY != "":
     full_no_proxy_list = []
     if data.values.TKG_NO_PROXY != "":
-      full_no_proxy_list = data.values.TKG_NO_PROXY.split(",")
+      # trim space in the no_proxy list
+      full_no_proxy_list = regexp.replace(" ", data.values.TKG_NO_PROXY, "").split(",")
     end
     if data.values.PROVIDER_TYPE == "aws":
-      full_no_proxy_list.append(data.values.AWS_VPC_CIDR)
+      if data.values.AWS_VPC_CIDR != "":
+        full_no_proxy_list.append(data.values.AWS_VPC_CIDR)
+      end
       full_no_proxy_list.append("169.254.0.0/16")
     end
     if data.values.PROVIDER_TYPE == "azure":
-      full_no_proxy_list.append(data.values.AZURE_VNET_CIDR)
+      if data.values.AZURE_VNET_CIDR != "":
+        full_no_proxy_list.append(data.values.AZURE_VNET_CIDR)
+      end
       full_no_proxy_list.append("169.254.0.0/16")
       full_no_proxy_list.append("168.63.129.16")
     end
@@ -310,7 +315,7 @@ def get_no_proxy():
     end
     full_no_proxy_list.append(".svc")
     full_no_proxy_list.append(".svc.cluster.local")
-    populated_no_proxy = ",".join(sorted(list(set(full_no_proxy_list))))
+    populated_no_proxy = ",".join(list(set(full_no_proxy_list)))
     return populated_no_proxy
   end
   return ""
@@ -331,7 +336,7 @@ end
 # get_labels_map_from_string constructs a map from given string of the format "key1=label1,key2=label2"
 def get_labels_map_from_string(labelString):
    labelMap = {}
-   for val in labelString.split(','):
+   for val in regexp.replace(" ", labelString, "").split(','):
     kv = val.split('=')
     if len(kv) != 2:
       assert.fail("given labels string \""+labelString+"\" must be in the  \"key1=label1,key2=label2\" format ")

pkg/v1/providers/yttcc/lib/helpers.star

@@ -1,5 +1,6 @@
 load("@ytt:data", "data")
 load("@ytt:assert", "assert")
+load("@ytt:regexp", "regexp")
 
 TKGSProductName = "VMware Tanzu Kubernetes Grid Service for vSphere"
 TKGProductName = "VMware Tanzu Kubernetes Grid"
@@ -277,14 +278,19 @@ def get_no_proxy():
   if data.values.TKG_HTTP_PROXY != "":
     full_no_proxy_list = []
     if data.values.TKG_NO_PROXY != "":
-      full_no_proxy_list = data.values.TKG_NO_PROXY.split(",")
+      # trim space in the no_proxy list
+      full_no_proxy_list = regexp.replace(" ", data.values.TKG_NO_PROXY, "").split(",")
     end
     if data.values.PROVIDER_TYPE == "aws":
-      full_no_proxy_list.append(data.values.AWS_VPC_CIDR)
+      if data.values.AWS_VPC_CIDR != "":
+        full_no_proxy_list.append(data.values.AWS_VPC_CIDR)
+      end
       full_no_proxy_list.append("169.254.0.0/16")
     end
     if data.values.PROVIDER_TYPE == "azure":
-      full_no_proxy_list.append(data.values.AZURE_VNET_CIDR)
+      if data.values.AZURE_VNET_CIDR != "":
+        full_no_proxy_list.append(data.values.AZURE_VNET_CIDR)
+      end
       full_no_proxy_list.append("169.254.0.0/16")
       full_no_proxy_list.append("168.63.129.16")
     end

pkg/v1/tkg/client/validate.go

@@ -1565,6 +1565,13 @@ func (c *TkgClient) getFullTKGNoProxy(providerName string) (string, error) {
 	}
 
 	if noProxy, _ := c.TKGConfigReaderWriter().Get(constants.TKGNoProxy); noProxy != "" {
+		// trim space
+		replaceSpacePattern := regexp.MustCompile(`\s+|\t+|\n+|\r+`)
+		noProxy = replaceSpacePattern.ReplaceAllString(noProxy, "")
+
+		if strings.Contains(noProxy, "*") {
+			return "", fmt.Errorf("invalid string '*' in %s", constants.TKGNoProxy)
+		}
 		for _, np := range strings.Split(noProxy, ",") {
 			noProxyMap[np] = true
 		}

pkg/v1/tkg/client/validate_test.go

@@ -370,6 +370,32 @@ var _ = Describe("Validate", func() {
 							Expect(validationError.Error()).To(ContainSubstring("invalid TKG_HTTPS_PROXY \"http://[::1]\", expected to be an address of type \"ipv4\" (TKG_IP_FAMILY)"))
 						})
 					})
+					DescribeTable("NO_PROXY validate", func(httpProxy, httpsProxy, noProxy string, hasError bool) {
+						tkgConfigReaderWriter.Set(constants.TKGHTTPProxy, httpProxy)
+						tkgConfigReaderWriter.Set(constants.TKGHTTPSProxy, httpsProxy)
+						tkgConfigReaderWriter.Set(constants.TKGHTTPProxyEnabled, "true")
+						tkgConfigReaderWriter.Set(constants.TKGNoProxy, noProxy)
+
+						validationError := tkgClient.ConfigureAndValidateManagementClusterConfiguration(initRegionOptions, true)
+						if hasError {
+							Expect(validationError).To(HaveOccurred())
+							return
+						}
+
+						Expect(validationError).NotTo(HaveOccurred())
+						v, err := tkgConfigReaderWriter.Get(constants.TKGNoProxy)
+						Expect(err).NotTo(HaveOccurred())
+						Expect(v).NotTo(ContainSubstring(" "))
+						Expect(v).NotTo(ContainSubstring("	"))
+						Expect(v).NotTo(ContainSubstring(`
+						`))
+					},
+						Entry("No proxy has new line, trim new line", "http://1.2.3.4", "http://1.2.3.4", `10.2.1.3/23,
+                			10.1.3.3`, false),
+						Entry("No Proxy has space, trim space", "http://1.2.3.4", "http://1.2.3.4", "example.com, svc.c", false),
+						Entry("No Proxy has *", "http://1.2.3.4", "http://1.2.3.4", "example.com, svc.c,*.vmware.com", true),
+						Entry("No Proxy", "http://1.2.3.4", "http://1.2.3.4", "10.0.0.0/24", false),
+					)
 				})
 			})