Sanitize port parsing for pods in play kube

[tylarb]

The logic used in parsing the ports to be utilized in a kubenetes api
defined pod did not fully adhere to the kubenetes spec, nor did it map
well to a podman context. This fix sanitizes the input of container
ports to meet the following rules:

• A defined containerPort with no defined hostPort does nothing in a
  podman context, or is informational. This is line with usage in
  Kubernetes.

• A defined hostPort with no defined containerPort acts like a
  publish [hostPort]:[hostPort]

• A defined containerPort and defined hostPort works like it does in
  kubernetes, as in a publish [hostPort]:[containerPort]

Addresses #5610

Signed-off-by: Tyler Ramer [email protected]

[openshift-ci-robot]

Hi @tylarb. Thanks for your PR.

I'm waiting for a containers member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[tylarb]

/cc @giuseppe @mheon

[mheon]

I feel like this might be better handled inside play kube as opposed to more generic pod code, as we don't necessarily want podman pod create -p 80:0 to work

[tylarb]

@mheon actually... podman pod create -p 80:0 does currently work (or doesn't work, depending on how you look at it)

libpod$ podman pod create -p 80:0 --name test
9c3c794006a30b475ae71487de91d7c899deedd0d92abeba62a08ed75b8c93da
libpod$ podman ps
CONTAINER ID  IMAGE  COMMAND  CREATED  STATUS  PORTS  NAMES
libpod$ podman pod ls
POD ID         NAME   STATUS    CREATED          # OF CONTAINERS   INFRA ID
9c3c794006a3   test   Created   13 seconds ago   1                 f615633b85a4
libpod$ podman version 
Version:            1.8.2
RemoteAPI Version:  1
Go Version:         go1.14
Git Commit:         028e3317eb1494b9b2acba4a0a295df80fae66cc
Built:              Sat Mar 21 09:30:34 2020
OS/Arch:            linux/amd64
libpod$ podman run --pod test  --rm -d quay.io/redhatworkshops/simple-python-web:latest /usr/bin/python3 -m http.server 8080
ERRO[0004] error starting some container dependencies   
ERRO[0004] "failed to expose ports via rootlessport: \"invalid ChildPort: '\\\\x00'\\n\"" 
Error: error starting some containers: internal libpod error

Also, getPodPorts is only called from inside PlayKubeYAML :

libpod(5610_play_kube_network)$ grep -Rn getPodPorts . 
./pkg/adapter/pods.go:595:	podPorts := getPodPorts(podYAML.Spec.Containers)
./pkg/adapter/pods.go:765:// getPodPorts converts a slice of kube container descriptions to an
./pkg/adapter/pods.go:767:func getPodPorts(containers []v1.Container) []ocicni.PortMapping {

 540 // PlayKubeYAML creates pods and containers from a kube YAML file
 541 func (r *LocalRuntime) PlayKubeYAML(ctx context.Context, c *cliconfig.KubePlayValues, yamlFile string) (*Pod, error) {
...
 580 ⇥       hostname := podYAML.Spec.Hostname
 581 ⇥       if hostname == "" {
 582 ⇥       ⇥       hostname = podName
 583 ⇥       }
 584 ⇥       podOptions = append(podOptions, libpod.WithPodHostname(hostname))
 585 
 586 ⇥       if podYAML.Spec.HostNetwork {
 587 ⇥       ⇥       podOptions = append(podOptions, libpod.WithPodHostNetwork())
 588 ⇥       }
 589 
 590 ⇥       nsOptions, err := shared.GetNamespaceOptions(strings.Split(shared.DefaultKernelNamespaces, ","))
 591 ⇥       if err != nil {
 592 ⇥       ⇥       return nil, err
 593 ⇥       }
 594 ⇥       podOptions = append(podOptions, nsOptions...)
 595 ⇥       podPorts := getPodPorts(podYAML.Spec.Containers)
 596 ⇥       podOptions = append(podOptions, libpod.WithInfraContainerPorts(podPorts))

[mheon]

Apparently our port validation isn't as good as I thought... But that's something for another time.

If this function is only called from within play kube we ought to be fine.

/approve
/ok-to-test

[mheon]

LGTM

[rhatdan]

/lgtm

[rhatdan]

/approve

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mheon, rhatdan, tylarb

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [mheon,rhatdan]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment