Skip to content

user: add special container user constants for windows to sys/user#184

Closed
jsternberg wants to merge 1 commit intomoby:mainfrom
jsternberg:windows-container-users
Closed

user: add special container user constants for windows to sys/user#184
jsternberg wants to merge 1 commit intomoby:mainfrom
jsternberg:windows-container-users

Conversation

@jsternberg
Copy link

No description provided.

Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
@crazy-max
Copy link
Member

relates to moby/buildkit#5791 (review)

@kolyshkin
Copy link
Collaborator

It would be nice to at least add a comment telling something about where these are coming from (I know they are not documented etc. -- but at least provide some sources).

@thaJeztah
Copy link
Member

Let me leave a "request changes", because i don't think we'd be merging this currently.

As I commented on other PRs, I think these "magic" users should be considered a contract for Windows containers. And given that there's only 1 party that provides Windows base images, those should be considered a specification, and these users to be a part of that.

So what I would like to see;

  • Microsoft to include these somewhere in their official documentation
  • Given that they're defined in Microsoft/hcsshim; a set of properly documented consts (or similar) outlining these users (and their purpose)
  • Ideally, the OCI specification to also include these (as, per the above, it's effectively a specification for Windows images)

I want to avoid Moby, or BuildKit to become the "source of truth" for these consts, because we don't own this specification, and we don't want others to depend on those, so preferably keep them either internal, or IF we export them, to be documented with a reference to the canonical source of truth / specification.

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see above

@jsternberg jsternberg closed this Apr 4, 2025
@jsternberg
Copy link
Author

We merged the version of this that has these constants in buildkit. I think @thaJeztah is correct that this should go somewhere else and buildkit is a good place to put it temporarily.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants