The functionality is used by Docker clients like CLI and buildx along with Docker backend daemon for API 1.24 support of security options in system info responses.

We should probably have a look at some of this; for the CLI, it looks like it's purely for presenting docker info; not sure how buildx uses it. It's slightly odd that we have an API response that gets unmarshaled to a type defined in the API that's.. not strictly the type that it returns 🤔 (even wondering if there should be some unmarshal on the system.Info 🤔

(no good answers here, just thinking out loud)

The functionality is used by Docker clients like CLI and buildx along with Docker backend daemon for API 1.24 support of security options in system info responses.

We should probably have a look at some of this; for the CLI, it looks like it's purely for presenting docker info; not sure how buildx uses it. It's slightly odd that we have an API response that gets unmarshaled to a type defined in the API that's.. not strictly the type that it returns 🤔 (even wondering if there should be some unmarshal on the system.Info 🤔

(no good answers here, just thinking out loud)

+1, I wasn't sure about this one. So tried something out thinking others would give feedback for a better idea.

From https://github.com/docker/buildx/blob/177b9809583d03ea4af637c170716c739f7e4762/driver/docker-container/driver.go#L198, it looks like buildx uses it to detect if a container needs to be created with host userns mode.

@thaJeztah , some options that come to mind after some thought. Thoughts?

1. Info.DecodeSecurityOptions so the function is at least on a type defined in the package. Positives: Minimal user changes. (Can even deprecate and stair step users to upgrade) Negatives: Really just ignores the problem.
2. Make it client's responsibility to parse. Positives: Known users are Docker CLI and Buildx so pretty easy to make those changes. Negatives: Throwing responsibility over the wall.
3. API change to have the type of SecurityOptions be the type users actually want. (Is this possible without being a breaking change?)

Offline discussion with @thaJeztah and @corhere on this. The thought is to move the functionality into the client (and therefore also a copy into the backend) separating the shape of the data from the semantics of how it is used.

As follow-up work, it would be nice to make secopts in the API return as unmarshalled type but that should be looked at seperately.

Thanks! Overall, this LGTM; one quick comment before we continue;

As follow-up work, it would be nice to make secopts in the API return as unmarshalled type but that should be looked at seperately.

The only thing I'm considering is if we want to use the existing SecurityOpt struct for that (in which case possibly we should keep it in the API module), or if we want to design that format "from scratch" and define a new type once we do work on that.

@austinvazquez @corhere any thoughts?

Perhaps it's fine to do "from scratch"; the current type isn't overly creative, so perhaps that's the best option, but just in case 😅

The only thing I'm considering is if we want to use the existing SecurityOpt struct for that (in which case possibly we should keep it in the API module), or if we want to design that format "from scratch" and define a new type once we do work on that.

Perhaps it's fine to do "from scratch"; the current type isn't overly creative, so perhaps that's the best option, but just in case 😅

I think it's fine to merge as is and reconsider what we want to do with Info.SecurityOptions later on. At least, with the type moved out of the api module, we're free to invent something else, maybe with the same name, and shape it however we want.

Thanks @akerouanton

OK; let's bring this in 👍

I think it's fine to merge as is and reconsider what we want to do with Info.SecurityOptions later on. At least, with the type moved out of the api module, we're free to invent something else, maybe with the same name, and shape it however we want.

Yep, aligned with this.