From 2f081075a8768cd9ec62e82e0a30263e1aed04f2 Mon Sep 17 00:00:00 2001 From: ichx Date: Sun, 29 Aug 2021 21:37:22 +0800 Subject: [PATCH] Fix misuse in struct kobject.ServiceConfig.Port (#1423) --- pkg/kobject/kobject.go | 4 +- pkg/loader/bundle/bundle.go | 19 ++---- pkg/loader/compose/compose_test.go | 66 +++++++++---------- pkg/loader/compose/v1v2.go | 12 ++-- pkg/loader/compose/v3.go | 12 ++-- pkg/transformer/kubernetes/k8sutils_test.go | 12 ++-- pkg/transformer/kubernetes/kubernetes.go | 41 +++++------- pkg/transformer/kubernetes/kubernetes_test.go | 4 +- pkg/transformer/openshift/openshift_test.go | 2 +- 9 files changed, 80 insertions(+), 92 deletions(-) diff --git a/pkg/kobject/kobject.go b/pkg/kobject/kobject.go index 35b7b112..6d42c344 100644 --- a/pkg/kobject/kobject.go +++ b/pkg/kobject/kobject.go @@ -26,7 +26,6 @@ import ( "github.com/pkg/errors" "github.com/spf13/cast" v1 "k8s.io/api/apps/v1" - corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/util/intstr" ) @@ -91,6 +90,7 @@ func (opt *ConvertOptions) IsPodController() bool { type ServiceConfigGroup []ServiceConfig // ServiceConfig holds the basic struct of a container +// which should not introduce any kubernetes specific struct type ServiceConfig struct { Name string ContainerName string @@ -186,7 +186,7 @@ type Ports struct { HostPort int32 ContainerPort int32 HostIP string - Protocol corev1.Protocol + Protocol string // Upper string } // Volumes holds the volume struct of container diff --git a/pkg/loader/bundle/bundle.go b/pkg/loader/bundle/bundle.go index 9b61018d..93988c0e 100644 --- a/pkg/loader/bundle/bundle.go +++ b/pkg/loader/bundle/bundle.go @@ -156,20 +156,15 @@ func loadEnvVars(service Service) ([]kobject.EnvVar, error) { func loadPorts(service Service) ([]kobject.Ports, error) { ports := []kobject.Ports{} for _, port := range service.Ports { - var p api.Protocol - switch port.Protocol { - default: - p = api.ProtocolTCP - case "TCP": - p = api.ProtocolTCP - case "UDP": - p = api.ProtocolUDP - } - ports = append(ports, kobject.Ports{ + komposePorts := kobject.Ports{ HostPort: int32(port.Port), ContainerPort: int32(port.Port), - Protocol: p, - }) + Protocol: port.Protocol, + } + if protocol := api.Protocol(port.Protocol); protocol != api.ProtocolTCP && protocol != api.ProtocolUDP { + komposePorts.Protocol = string(api.ProtocolTCP) + } + ports = append(ports, komposePorts) } return ports, nil } diff --git a/pkg/loader/compose/compose_test.go b/pkg/loader/compose/compose_test.go index 07653c61..aed7a518 100644 --- a/pkg/loader/compose/compose_test.go +++ b/pkg/loader/compose/compose_test.go @@ -124,20 +124,20 @@ func TestLoadV3Ports(t *testing.T) { }{ { desc: "ports with expose", - ports: []types.ServicePortConfig{{Target: 80, Published: 80, Protocol: "TCP"}}, + ports: []types.ServicePortConfig{{Target: 80, Published: 80, Protocol: string(api.ProtocolTCP)}}, expose: []string{"80", "8080"}, want: []kobject.Ports{ - {HostPort: 80, ContainerPort: 80, Protocol: api.Protocol("TCP")}, - {HostPort: 8080, ContainerPort: 8080, Protocol: api.Protocol("TCP")}, + {HostPort: 80, ContainerPort: 80, Protocol: string(api.ProtocolTCP)}, + {HostPort: 8080, ContainerPort: 8080, Protocol: string(api.ProtocolTCP)}, }, }, { desc: "exposed port including /protocol", - ports: []types.ServicePortConfig{{Target: 80, Published: 80, Protocol: "TCP"}}, + ports: []types.ServicePortConfig{{Target: 80, Published: 80, Protocol: string(api.ProtocolTCP)}}, expose: []string{"80/udp"}, want: []kobject.Ports{ - {HostPort: 80, ContainerPort: 80, Protocol: api.Protocol("TCP")}, - {HostPort: 80, ContainerPort: 80, Protocol: api.Protocol("UDP")}, + {HostPort: 80, ContainerPort: 80, Protocol: string(api.ProtocolTCP)}, + {HostPort: 80, ContainerPort: 80, Protocol: string(api.ProtocolUDP)}, }, }, } { @@ -187,74 +187,74 @@ func TestLoadPorts(t *testing.T) { { ports: []string{"127.0.0.1:80:80/tcp"}, want: []kobject.Ports{ - {HostIP: "127.0.0.1", HostPort: 80, ContainerPort: 80, Protocol: api.ProtocolTCP}, + {HostIP: "127.0.0.1", HostPort: 80, ContainerPort: 80, Protocol: string(api.ProtocolTCP)}, }, }, { ports: []string{"80:80/tcp"}, want: []kobject.Ports{ - {HostPort: 80, ContainerPort: 80, Protocol: api.ProtocolTCP}, + {HostPort: 80, ContainerPort: 80, Protocol: string(api.ProtocolTCP)}, }, }, { ports: []string{"80:80"}, want: []kobject.Ports{ - {HostPort: 80, ContainerPort: 80, Protocol: api.ProtocolTCP}, + {HostPort: 80, ContainerPort: 80, Protocol: string(api.ProtocolTCP)}, }, }, { ports: []string{"80"}, want: []kobject.Ports{ - {ContainerPort: 80, Protocol: api.ProtocolTCP}, + {ContainerPort: 80, Protocol: string(api.ProtocolTCP)}, }, }, { ports: []string{"3000-3005"}, want: []kobject.Ports{ - {ContainerPort: 3000, Protocol: api.ProtocolTCP}, - {ContainerPort: 3001, Protocol: api.ProtocolTCP}, - {ContainerPort: 3002, Protocol: api.ProtocolTCP}, - {ContainerPort: 3003, Protocol: api.ProtocolTCP}, - {ContainerPort: 3004, Protocol: api.ProtocolTCP}, - {ContainerPort: 3005, Protocol: api.ProtocolTCP}, + {ContainerPort: 3000, Protocol: string(api.ProtocolTCP)}, + {ContainerPort: 3001, Protocol: string(api.ProtocolTCP)}, + {ContainerPort: 3002, Protocol: string(api.ProtocolTCP)}, + {ContainerPort: 3003, Protocol: string(api.ProtocolTCP)}, + {ContainerPort: 3004, Protocol: string(api.ProtocolTCP)}, + {ContainerPort: 3005, Protocol: string(api.ProtocolTCP)}, }, }, { ports: []string{"3000-3005:5000-5005"}, want: []kobject.Ports{ - {HostPort: 3000, ContainerPort: 5000, Protocol: api.ProtocolTCP}, - {HostPort: 3001, ContainerPort: 5001, Protocol: api.ProtocolTCP}, - {HostPort: 3002, ContainerPort: 5002, Protocol: api.ProtocolTCP}, - {HostPort: 3003, ContainerPort: 5003, Protocol: api.ProtocolTCP}, - {HostPort: 3004, ContainerPort: 5004, Protocol: api.ProtocolTCP}, - {HostPort: 3005, ContainerPort: 5005, Protocol: api.ProtocolTCP}, + {HostPort: 3000, ContainerPort: 5000, Protocol: string(api.ProtocolTCP)}, + {HostPort: 3001, ContainerPort: 5001, Protocol: string(api.ProtocolTCP)}, + {HostPort: 3002, ContainerPort: 5002, Protocol: string(api.ProtocolTCP)}, + {HostPort: 3003, ContainerPort: 5003, Protocol: string(api.ProtocolTCP)}, + {HostPort: 3004, ContainerPort: 5004, Protocol: string(api.ProtocolTCP)}, + {HostPort: 3005, ContainerPort: 5005, Protocol: string(api.ProtocolTCP)}, }, }, { ports: []string{"127.0.0.1:3000-3005:5000-5005"}, want: []kobject.Ports{ - {HostIP: "127.0.0.1", HostPort: 3000, ContainerPort: 5000, Protocol: api.ProtocolTCP}, - {HostIP: "127.0.0.1", HostPort: 3001, ContainerPort: 5001, Protocol: api.ProtocolTCP}, - {HostIP: "127.0.0.1", HostPort: 3002, ContainerPort: 5002, Protocol: api.ProtocolTCP}, - {HostIP: "127.0.0.1", HostPort: 3003, ContainerPort: 5003, Protocol: api.ProtocolTCP}, - {HostIP: "127.0.0.1", HostPort: 3004, ContainerPort: 5004, Protocol: api.ProtocolTCP}, - {HostIP: "127.0.0.1", HostPort: 3005, ContainerPort: 5005, Protocol: api.ProtocolTCP}, + {HostIP: "127.0.0.1", HostPort: 3000, ContainerPort: 5000, Protocol: string(api.ProtocolTCP)}, + {HostIP: "127.0.0.1", HostPort: 3001, ContainerPort: 5001, Protocol: string(api.ProtocolTCP)}, + {HostIP: "127.0.0.1", HostPort: 3002, ContainerPort: 5002, Protocol: string(api.ProtocolTCP)}, + {HostIP: "127.0.0.1", HostPort: 3003, ContainerPort: 5003, Protocol: string(api.ProtocolTCP)}, + {HostIP: "127.0.0.1", HostPort: 3004, ContainerPort: 5004, Protocol: string(api.ProtocolTCP)}, + {HostIP: "127.0.0.1", HostPort: 3005, ContainerPort: 5005, Protocol: string(api.ProtocolTCP)}, }, }, { ports: []string{"80", "3000"}, want: []kobject.Ports{ - {HostPort: 0, ContainerPort: 80, Protocol: api.ProtocolTCP}, - {HostPort: 0, ContainerPort: 3000, Protocol: api.ProtocolTCP}, + {HostPort: 0, ContainerPort: 80, Protocol: string(api.ProtocolTCP)}, + {HostPort: 0, ContainerPort: 3000, Protocol: string(api.ProtocolTCP)}, }, }, { ports: []string{"80", "3000"}, expose: []string{"80", "8080"}, want: []kobject.Ports{ - {HostPort: 0, ContainerPort: 80, Protocol: api.ProtocolTCP}, - {HostPort: 0, ContainerPort: 3000, Protocol: api.ProtocolTCP}, - {HostPort: 0, ContainerPort: 8080, Protocol: api.ProtocolTCP}, + {HostPort: 0, ContainerPort: 80, Protocol: string(api.ProtocolTCP)}, + {HostPort: 0, ContainerPort: 3000, Protocol: string(api.ProtocolTCP)}, + {HostPort: 0, ContainerPort: 8080, Protocol: string(api.ProtocolTCP)}, }, }, } diff --git a/pkg/loader/compose/v1v2.go b/pkg/loader/compose/v1v2.go index 9697c539..a61ae1e0 100644 --- a/pkg/loader/compose/v1v2.go +++ b/pkg/loader/compose/v1v2.go @@ -132,7 +132,7 @@ func loadPorts(composePorts []string, expose []string) ([]kobject.Ports, error) HostPort: int32(cfg.PublishedPort), ContainerPort: int32(cfg.TargetPort), HostIP: hostIP, - Protocol: api.Protocol(strings.ToUpper(string(cfg.Protocol))), + Protocol: strings.ToUpper(string(cfg.Protocol)), }) } } @@ -141,23 +141,23 @@ func loadPorts(composePorts []string, expose []string) ([]kobject.Ports, error) // load remain expose ports for _, p := range kp { // must use cast... - exist[cast.ToString(p.ContainerPort)+string(p.Protocol)] = true + exist[cast.ToString(p.ContainerPort)+p.Protocol] = true } if expose != nil { for _, port := range expose { portValue := port - protocol := api.ProtocolTCP + protocol := string(api.ProtocolTCP) if strings.Contains(portValue, "/") { splits := strings.Split(port, "/") portValue = splits[0] - protocol = api.Protocol(strings.ToUpper(splits[1])) + protocol = splits[1] } - if !exist[portValue+string(protocol)] { + if !exist[portValue+protocol] { kp = append(kp, kobject.Ports{ ContainerPort: cast.ToInt32(portValue), - Protocol: protocol, + Protocol: strings.ToUpper(protocol), }) } } diff --git a/pkg/loader/compose/v3.go b/pkg/loader/compose/v3.go index 1e00a948..8ba96add 100644 --- a/pkg/loader/compose/v3.go +++ b/pkg/loader/compose/v3.go @@ -210,30 +210,30 @@ func loadV3Ports(ports []types.ServicePortConfig, expose []string) []kobject.Por HostPort: int32(port.Published), ContainerPort: int32(port.Target), HostIP: "", - Protocol: api.Protocol(strings.ToUpper(port.Protocol)), + Protocol: strings.ToUpper(port.Protocol), }) - exist[cast.ToString(port.Target)+strings.ToUpper(port.Protocol)] = true + exist[cast.ToString(port.Target)+port.Protocol] = true } if expose != nil { for _, port := range expose { portValue := port - protocol := api.ProtocolTCP + protocol := string(api.ProtocolTCP) if strings.Contains(portValue, "/") { splits := strings.Split(port, "/") portValue = splits[0] - protocol = api.Protocol(strings.ToUpper(splits[1])) + protocol = splits[1] } - if exist[portValue+string(protocol)] { + if exist[portValue+protocol] { continue } komposePorts = append(komposePorts, kobject.Ports{ HostPort: cast.ToInt32(portValue), ContainerPort: cast.ToInt32(portValue), HostIP: "", - Protocol: protocol, + Protocol: strings.ToUpper(protocol), }) } } diff --git a/pkg/transformer/kubernetes/k8sutils_test.go b/pkg/transformer/kubernetes/k8sutils_test.go index 134251ab..799c7699 100644 --- a/pkg/transformer/kubernetes/k8sutils_test.go +++ b/pkg/transformer/kubernetes/k8sutils_test.go @@ -40,7 +40,7 @@ func TestCreateService(t *testing.T) { ContainerName: "name", Image: "image", Environment: []kobject.EnvVar{kobject.EnvVar{Name: "env", Value: "value"}}, - Port: []kobject.Ports{kobject.Ports{HostPort: 123, ContainerPort: 456, Protocol: corev1.ProtocolTCP}}, + Port: []kobject.Ports{kobject.Ports{HostPort: 123, ContainerPort: 456, Protocol: string(corev1.ProtocolTCP)}}, Command: []string{"cmd"}, WorkingDir: "dir", Args: []string{"arg1", "arg2"}, @@ -83,7 +83,7 @@ func TestCreateServiceWithMemLimit(t *testing.T) { ContainerName: "name", Image: "image", Environment: []kobject.EnvVar{kobject.EnvVar{Name: "env", Value: "value"}}, - Port: []kobject.Ports{kobject.Ports{HostPort: 123, ContainerPort: 456, Protocol: corev1.ProtocolTCP}}, + Port: []kobject.Ports{kobject.Ports{HostPort: 123, ContainerPort: 456, Protocol: string(corev1.ProtocolTCP)}}, Command: []string{"cmd"}, WorkingDir: "dir", Args: []string{"arg1", "arg2"}, @@ -135,7 +135,7 @@ func TestCreateServiceWithCPULimit(t *testing.T) { ContainerName: "name", Image: "image", Environment: []kobject.EnvVar{kobject.EnvVar{Name: "env", Value: "value"}}, - Port: []kobject.Ports{kobject.Ports{HostPort: 123, ContainerPort: 456, Protocol: corev1.ProtocolTCP}}, + Port: []kobject.Ports{kobject.Ports{HostPort: 123, ContainerPort: 456, Protocol: string(corev1.ProtocolTCP)}}, Command: []string{"cmd"}, WorkingDir: "dir", Args: []string{"arg1", "arg2"}, @@ -188,7 +188,7 @@ func TestCreateServiceWithServiceUser(t *testing.T) { ContainerName: "name", Image: "image", Environment: []kobject.EnvVar{kobject.EnvVar{Name: "env", Value: "value"}}, - Port: []kobject.Ports{kobject.Ports{HostPort: 123, ContainerPort: 456, Protocol: corev1.ProtocolTCP}}, + Port: []kobject.Ports{kobject.Ports{HostPort: 123, ContainerPort: 456, Protocol: string(corev1.ProtocolTCP)}}, Command: []string{"cmd"}, WorkingDir: "dir", Args: []string{"arg1", "arg2"}, @@ -231,7 +231,7 @@ func TestTransformWithPid(t *testing.T) { ContainerName: "name", Image: "image", Environment: []kobject.EnvVar{kobject.EnvVar{Name: "env", Value: "value"}}, - Port: []kobject.Ports{kobject.Ports{HostPort: 123, ContainerPort: 456, Protocol: corev1.ProtocolTCP}}, + Port: []kobject.Ports{kobject.Ports{HostPort: 123, ContainerPort: 456, Protocol: string(corev1.ProtocolTCP)}}, Command: []string{"cmd"}, WorkingDir: "dir", Args: []string{"arg1", "arg2"}, @@ -267,7 +267,7 @@ func TestTransformWithInvalidPid(t *testing.T) { ContainerName: "name", Image: "image", Environment: []kobject.EnvVar{kobject.EnvVar{Name: "env", Value: "value"}}, - Port: []kobject.Ports{kobject.Ports{HostPort: 123, ContainerPort: 456, Protocol: corev1.ProtocolTCP}}, + Port: []kobject.Ports{kobject.Ports{HostPort: 123, ContainerPort: 456, Protocol: string(corev1.ProtocolTCP)}}, Command: []string{"cmd"}, WorkingDir: "dir", Args: []string{"arg1", "arg2"}, diff --git a/pkg/transformer/kubernetes/kubernetes.go b/pkg/transformer/kubernetes/kubernetes.go index 9c461c18..e8015302 100644 --- a/pkg/transformer/kubernetes/kubernetes.go +++ b/pkg/transformer/kubernetes/kubernetes.go @@ -574,23 +574,19 @@ func ConfigPorts(name string, service kobject.ServiceConfig) []api.ContainerPort exist := map[string]bool{} for _, port := range service.Port { // temp use as an id - if exist[string(port.ContainerPort)+string(port.Protocol)] { + if exist[string(port.ContainerPort)+port.Protocol] { continue } - // If the default is already TCP, no need to include it. - if port.Protocol == api.ProtocolTCP { - ports = append(ports, api.ContainerPort{ - ContainerPort: port.ContainerPort, - HostIP: port.HostIP, - }) - } else { - ports = append(ports, api.ContainerPort{ - ContainerPort: port.ContainerPort, - Protocol: port.Protocol, - HostIP: port.HostIP, - }) + containerPort := api.ContainerPort{ + ContainerPort: port.ContainerPort, + HostIP: port.HostIP, } - exist[string(port.ContainerPort)+string(port.Protocol)] = true + // If the default is already TCP, no need to include protocol. + if protocol := api.Protocol(port.Protocol); protocol != api.ProtocolTCP { + containerPort.Protocol = protocol + } + ports = append(ports, containerPort) + exist[string(port.ContainerPort)+port.Protocol] = true } return ports @@ -613,14 +609,11 @@ func (k *Kubernetes) ConfigLBServicePorts(name string, service kobject.ServiceCo TargetPort: targetPort, } - // If the default is already TCP, no need to include it. - if port.Protocol != api.ProtocolTCP { - servicePort.Protocol = port.Protocol - } - - if port.Protocol == api.ProtocolTCP { + if protocol := api.Protocol(port.Protocol); protocol == api.ProtocolTCP { + // If the default is already TCP, no need to include protocol. tcpPorts = append(tcpPorts, servicePort) } else { + servicePort.Protocol = protocol udpPorts = append(udpPorts, servicePort) } } @@ -649,7 +642,7 @@ func (k *Kubernetes) ConfigServicePorts(name string, service kobject.ServiceConf if service.ServiceType == string(api.ServiceTypeLoadBalancer) { log.Fatalf("Service %s of type LoadBalancer cannot use TCP and UDP for the same port", name) } - name = fmt.Sprintf("%s-%s", name, strings.ToLower(string(port.Protocol))) + name = fmt.Sprintf("%s-%s", name, strings.ToLower(port.Protocol)) } servicePort = api.ServicePort{ @@ -662,9 +655,9 @@ func (k *Kubernetes) ConfigServicePorts(name string, service kobject.ServiceConf servicePort.NodePort = service.NodePortPort } - // If the default is already TCP, no need to include it. - if port.Protocol != api.ProtocolTCP { - servicePort.Protocol = port.Protocol + // If the default is already TCP, no need to include protocol. + if protocol := api.Protocol(port.Protocol); protocol != api.ProtocolTCP { + servicePort.Protocol = protocol } servicePorts = append(servicePorts, servicePort) diff --git a/pkg/transformer/kubernetes/kubernetes_test.go b/pkg/transformer/kubernetes/kubernetes_test.go index 54941369..e60995af 100644 --- a/pkg/transformer/kubernetes/kubernetes_test.go +++ b/pkg/transformer/kubernetes/kubernetes_test.go @@ -44,7 +44,7 @@ func newServiceConfig() kobject.ServiceConfig { ContainerName: "name", Image: "image", Environment: []kobject.EnvVar{kobject.EnvVar{Name: "env", Value: "value"}}, - Port: []kobject.Ports{kobject.Ports{HostPort: 123, ContainerPort: 456}, kobject.Ports{HostPort: 123, ContainerPort: 456, Protocol: api.ProtocolUDP}}, + Port: []kobject.Ports{kobject.Ports{HostPort: 123, ContainerPort: 456}, kobject.Ports{HostPort: 123, ContainerPort: 456, Protocol: string(api.ProtocolUDP)}}, Command: []string{"cmd"}, WorkingDir: "dir", Args: []string{"arg1", "arg2"}, @@ -115,7 +115,7 @@ func equalPorts(kobjectPorts []kobject.Ports, k8sPorts []api.ContainerPort) bool for _, k8sPort := range k8sPorts { // FIXME: HostPort should be copied to container port //if port.HostPort == k8sPort.HostPort && port.Protocol == k8sPort.Protocol && port.ContainerPort == k8sPort.ContainerPort { - if port.Protocol == k8sPort.Protocol && port.ContainerPort == k8sPort.ContainerPort { + if port.Protocol == string(k8sPort.Protocol) && port.ContainerPort == k8sPort.ContainerPort { found = true } // Name and HostIp shouldn't be set diff --git a/pkg/transformer/openshift/openshift_test.go b/pkg/transformer/openshift/openshift_test.go index 709c30b5..2bd3a061 100644 --- a/pkg/transformer/openshift/openshift_test.go +++ b/pkg/transformer/openshift/openshift_test.go @@ -37,7 +37,7 @@ func newServiceConfig() kobject.ServiceConfig { ContainerName: "myfoobarname", Image: "image", Environment: []kobject.EnvVar{kobject.EnvVar{Name: "env", Value: "value"}}, - Port: []kobject.Ports{kobject.Ports{HostPort: 123, ContainerPort: 456, Protocol: corev1.ProtocolTCP}}, + Port: []kobject.Ports{kobject.Ports{HostPort: 123, ContainerPort: 456, Protocol: string(corev1.ProtocolTCP)}}, Command: []string{"cmd"}, WorkingDir: "dir", Args: []string{"arg1", "arg2"},