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

Add container ID validation #1580

Closed
lengrongfu opened this issue Feb 17, 2023 · 3 comments
Closed

Add container ID validation #1580

lengrongfu opened this issue Feb 17, 2023 · 3 comments

Comments

@lengrongfu
Copy link
Collaborator

Current youki not container ID validate, but runc is having, https://github.com/opencontainers/runc/blob/537645fd52e97e1e33f3b6b8d71397affada9779/libcontainer/factory_linux.go#L278
Can I add this logic for youki?

@YJDoc2
Copy link
Collaborator

YJDoc2 commented Feb 20, 2023

Hey @lengrongfu , can you give a bit more info on where this function will be used? For example, right now we validate the id to be non-zero length in the cli parsing. Similarly, where would these validation be used?

@lengrongfu
Copy link
Collaborator Author

We can add a logic to call validateID in build() and verify whether the container id is valid.

@YJDoc2
Copy link
Collaborator

YJDoc2 commented Feb 20, 2023

Ok, I guess that will help us keep compatibility with runc...
I'm not particularly happy with the fact that the format of ID is not formally defined (as per the code you linked states), which means we are essentially following runc , and taking their choices as is it.

That said, we are trying to be compatible with other runtimes, so it would be a good idea to validate id to be "correct" as per them.

Sure, you can go ahead and open a PR for this, thanks a lot for pointing this out, and showing interest in adding the change :)

@YJDoc2 YJDoc2 changed the title Add container ID validate Add container ID validation Feb 20, 2023
@yihuaf yihuaf closed this as completed Mar 3, 2023
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

No branches or pull requests

3 participants