Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

net.IP and net.HardwareAddr are represented incorrectly in the OpenAPI spec #14160

Closed
SoMuchForSubtlety opened this issue May 9, 2022 · 8 comments
Labels
kind/bug Categorizes issue or PR as related to a bug. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.

Comments

@SoMuchForSubtlety
Copy link
Contributor

SoMuchForSubtlety commented May 9, 2022

Is this a BUG REPORT or FEATURE REQUEST? (leave only one on its own line)

/kind bug

Description

The json response returned by PodInspectLibpod does not match the v4 OpenAPI schema. I'm using podman-api-rs and get the following error. invalid type: string "", expected a sequence at line 1 column 608.

This is the json response (formatted by me)

{
  "Id": "548ed026ce4e46bee640176a253a874eaef370af39f01ea91ae5819cf256eb61",
  "Name": "my-pod",
  "Created": "2022-04-17T19:22:15.92511409+02:00",
  "State": "Running",
  "Hostname": "my-pod",
  "CreateCgroup": true,
  "CgroupParent": "user.slice",
  "CgroupPath": "user.slice/user-libpod_pod_548ed026ce4e46bee640176a253a874eaef370af39f01ea91ae5819cf256eb61.slice",
  "CreateInfra": true,
  "InfraContainerID": "592793a958a240ef5f427c2e3844692ccd82a95b31b8af74a9369aeea10ccf5c",
  "InfraConfig": {
    "PortBindings": {
      "27017/tcp": [{ "HostIp": "", "HostPort": "27017" }],
      "5432/tcp": [{ "HostIp": "", "HostPort": "5432" }]
    },
    "HostNetwork": true,
    "StaticIP": "",
    "StaticMAC": "",
    "NoManageResolvConf": false,
    "DNSServer": null,
    "DNSSearch": null,
    "DNSOption": null,
    "NoManageHosts": false,
    "HostAdd": null,
    "Networks": null,
    "NetworkOptions": null,
    "pid_ns": "private",
    "userns": "host"
  },
  "SharedNamespaces": ["uts", "ipc", "net"],
  "NumContainers": 3,
  "Containers": [
    {
      "Id": "040e3fc98cfcf9eeab0e7122c7c37ae874025fae02355b91c27afb1e6913b5d2",
      "Name": "postgres",
      "State": "running"
    },
    {
      "Id": "592793a958a240ef5f427c2e3844692ccd82a95b31b8af74a9369aeea10ccf5c",
      "Name": "548ed026ce4e-infra",
      "State": "running"
    },
    {
      "Id": "8b601576bfdab76be7543eba91eed775875d0f27820be3eed5cf8109936e6996",
      "Name": "mongodb",
      "State": "running"
    }
  ]
}
"StaticIP": ""
            ^ line 1 column 608

The field InspectPodResponse.InfraConfig.StaticIP is incorrect. The expected type is []uint8, but its value is a string.

InspectPodResponse:
  description: Inspect pod
  schema:
    properties:
      # ... 
      InfraConfig:
        $ref: '#/definitions/InspectPodInfraConfig'
      # ... 

InspectPodInfraConfig:
    description: |-
      InspectPodInfraConfig contains the configuration of the pod's infra
      container.
    properties:
      # ...
      StaticIP:
        $ref: '#/definitions/IP'
      # ...

IP:
  description: |-
    Note that in this documentation, referring to an
    IP address as an IPv4 address or an IPv6 address
    is a semantic property of the address, not just the
    length of the byte slice: a 16-byte slice can still
    be an IPv4 address.
  items:
    format: uint8
    type: integer
  title: |-
    An IP is a single IP address, a slice of bytes.
    Functions in this package accept either 4-byte (IPv4)
    or 16-byte (IPv6) slices as input.
  type: array
  x-go-package: net

Steps to reproduce the issue:

  1. make a GET request to /libpod/pods/{name}/json
  2. try to unmarshal the response according to the OpenAPI schema
  3. unmarshalling fails due to type mismatch

Describe the results you received:

"StaticIP": ""

Describe the results you expected:

"StaticIP": [1,2,3,4] or a different OpenAPI schema

Additional information you deem important (e.g. issue happens only occasionally):

Output of podman version:

Client:       Podman Engine
Version:      4.0.3
API Version:  4.0.3
Go Version:   go1.18
Built:        Fri Apr  1 20:21:54 2022
OS/Arch:      linux/amd64

Output of podman info --debug:

host:
  arch: amd64
  buildahVersion: 1.24.3
  cgroupControllers:
  - cpu
  - io
  - memory
  - pids
  cgroupManager: systemd
  cgroupVersion: v2
  conmon:
    package: conmon-2.1.0-2.fc36.x86_64
    path: /usr/bin/conmon
    version: 'conmon version 2.1.0, commit: '
  cpus: 16
  distribution:
    distribution: fedora
    variant: workstation
    version: "36"
  eventLogger: journald
  hostname: honestmistake
  idMappings:
    gidmap:
    - container_id: 0
      host_id: 1000
      size: 1
    - container_id: 1
      host_id: 100000
      size: 65536
    uidmap:
    - container_id: 0
      host_id: 1000
      size: 1
    - container_id: 1
      host_id: 100000
      size: 65536
  kernel: 5.17.3-302.fc36.x86_64
  linkmode: dynamic
  logDriver: journald
  memFree: 927330304
  memTotal: 32904192000
  networkBackend: netavark
  ociRuntime:
    name: crun
    package: crun-1.4.4-1.fc36.x86_64
    path: /usr/bin/crun
    version: |-
      crun version 1.4.4
      commit: 6521fcc5806f20f6187eb933f9f45130c86da230
      spec: 1.0.0
      +SYSTEMD +SELINUX +APPARMOR +CAP +SECCOMP +EBPF +CRIU +YAJL
  os: linux
  remoteSocket:
    exists: true
    path: /run/user/1000/podman/podman.sock
  security:
    apparmorEnabled: false
    capabilities: CAP_CHOWN,CAP_DAC_OVERRIDE,CAP_FOWNER,CAP_FSETID,CAP_KILL,CAP_NET_BIND_SERVICE,CAP_SETFCAP,CAP_SETGID,CAP_SETPCAP,CAP_SETUID,CAP_SYS_CHROOT
    rootless: true
    seccompEnabled: true
    seccompProfilePath: /usr/share/containers/seccomp.json
    selinuxEnabled: true
  serviceIsRemote: false
  slirp4netns:
    executable: /usr/bin/slirp4netns
    package: slirp4netns-1.2.0-0.2.beta.0.fc36.x86_64
    version: |-
      slirp4netns version 1.2.0-beta.0
      commit: 477db14a24ff1a3de3a705e51ca2c4c1fe3dda64
      libslirp: 4.6.1
      SLIRP_CONFIG_VERSION_MAX: 3
      libseccomp: 2.5.3
  swapFree: 5330259968
  swapTotal: 8589930496
  uptime: 164h 15m 11.44s (Approximately 6.83 days)
plugins:
  log:
  - k8s-file
  - none
  - passthrough
  - journald
  network:
  - bridge
  - macvlan
  volume:
  - local
registries:
  search:
  - registry.fedoraproject.org
  - registry.access.redhat.com
  - docker.io
  - quay.io
store:
  configFile: /home/jakob/.config/containers/storage.conf
  containerStore:
    number: 6
    paused: 0
    running: 6
    stopped: 0
  graphDriverName: overlay
  graphOptions: {}
  graphRoot: /home/jakob/.local/share/containers/storage
  graphStatus:
    Backing Filesystem: btrfs
    Native Overlay Diff: "true"
    Supports d_type: "true"
    Using metacopy: "false"
  imageCopyTmpDir: /var/tmp
  imageStore:
    number: 9
  runRoot: /run/user/1000/containers
  volumePath: /home/jakob/.local/share/containers/storage/volumes
version:
  APIVersion: 4.0.3
  Built: 1648837314
  BuiltTime: Fri Apr  1 20:21:54 2022
  GitCommit: ""
  GoVersion: go1.18
  OsArch: linux/amd64
  Version: 4.0.3

Package info (e.g. output of rpm -q podman or apt list podman):

podman-4.0.3-1.fc36.x86_64

Have you tested with the latest version of Podman and have you checked the Podman Troubleshooting Guide? (https://github.com/containers/podman/blob/main/troubleshooting.md)

No

@openshift-ci openshift-ci bot added the kind/bug Categorizes issue or PR as related to a bug. label May 9, 2022
@mheon
Copy link
Member

mheon commented May 9, 2022

@jwhonce PTAL - looks like an error with the API docs

@SoMuchForSubtlety
Copy link
Contributor Author

SoMuchForSubtlety commented May 22, 2022

I'd be willing to implement a fix, but I need a few pointers on the best way to approach this.

The simplest fix would be to change the struct field1 to a string, but that would be a breaking change, and there are other references to #/definitions/IP that would remain broken.

While investigating this, I found other suspicious fields. StaticMAC2 in InspectPodInfraConfig is as far as I can tell never set, but it's defined as a string. Other structs use a custom wrapper type around net.HardwareAddr3, but the resulting OpenAPI model is incorrect (uint8[] again).

MacAddress:
  $ref: '#/definitions/HardwareAddr'
  description: |-
    HardwareAddr is the same as net.HardwareAddr except
    that it adds the json marshal/unmarshal methods.
    This allows us to read the mac from a json string
    and a byte array.

HardwareAddr:
  items:
    format: uint8
    type: integer
  title: A HardwareAddr represents a physical hardware address.
  type: array
  x-go-package: net

Footnotes

  1. https://github.com/containers/podman/blob/5c51e1d26ea7cb942008cf0e740bc16cb44bb2cf/libpod/define/pod_inspect.go#L85

  2. https://github.com/containers/podman/blob/5c51e1d26ea7cb942008cf0e740bc16cb44bb2cf/libpod/define/pod_inspect.go#L88

  3. https://github.com/containers/podman/blob/5c51e1d26ea7cb942008cf0e740bc16cb44bb2cf/vendor/github.com/containers/common/libnetwork/types/network.go#L102-L107

@mheon
Copy link
Member

mheon commented May 23, 2022

I would consider the current API correct - it's the documentation that is incorrect. As such I would advise against changing any struct fields.

The complexity here is that net.IP is a []uint8 but marshals into JSON as a string.

@SoMuchForSubtlety
Copy link
Contributor Author

I would consider the current API correct - it's the documentation that is incorrect. As such I would advise against changing any struct fields.

The complexity here is that net.IP is a []uint8 but marshals into JSON as a string.

Agreed, but as far as I can tell, there is currently no way to overwrite the type of a struct field with go-swagger. There is an open issue for this feature go-swagger/go-swagger#2599

@SoMuchForSubtlety
Copy link
Contributor Author

The swagger:strfmt hint seems to do the trick in this case, but only because StaticIP is explicitly defined as IPv4. There are eight other references to #/definitions/IP that still need to be addressed.

diff --git a/libpod/define/pod_inspect.go b/libpod/define/pod_inspect.go
index 219ffade2..c387856e5 100644
--- a/libpod/define/pod_inspect.go
+++ b/libpod/define/pod_inspect.go
@@ -82,6 +82,7 @@ type InspectPodInfraConfig struct {
 	HostNetwork bool
 	// StaticIP is a static IPv4 that will be assigned to the infra
 	// container and then used by the pod.
+	// swagger:strfmt ipv4
 	StaticIP net.IP
 	// StaticMAC is a static MAC address that will be assigned to the infra
 	// container and then used by the pod.

@rhatdan
Copy link
Member

rhatdan commented May 25, 2022

Care to open a PR to make these changes?

@SoMuchForSubtlety SoMuchForSubtlety changed the title /libpod/pods/{name}/json response does not match InspectPodResponse schema net.IP and net.HardwareAddr are represented incorrectly in the OpenAPI spec May 27, 2022
@github-actions
Copy link

A friendly reminder that this issue had no activity for 30 days.

@rhatdan
Copy link
Member

rhatdan commented Jun 27, 2022

Since a fix for this was merged, I am going to close.

@rhatdan rhatdan closed this as completed Jun 27, 2022
@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 20, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
kind/bug Categorizes issue or PR as related to a bug. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.
Projects
None yet
Development

No branches or pull requests

3 participants