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

Do not validate paths on remote platforms #166

Merged
merged 1 commit into from
May 28, 2020

Conversation

rhatdan
Copy link
Member

@rhatdan rhatdan commented May 28, 2020

Modify validate functions to work on a remote clients.
Any of the path checks will not work on remote machines or make
sense on remote clients. Therefore they should not be checked.

Signed-off-by: Daniel J Walsh [email protected]

@rhatdan
Copy link
Member Author

rhatdan commented May 28, 2020

@baude PTAL

@rhatdan
Copy link
Member Author

rhatdan commented May 28, 2020

Copy link
Member

@giuseppe giuseppe left a comment

Choose a reason for hiding this comment

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

LGTM

func (c *EngineConfig) validatePaths() error {
// Relative paths can cause nasty bugs, because core paths we use could
// shift between runs (or even parts of the program - the OCI runtime
// uses a different working directory than we do, for example.
Copy link
Member

Choose a reason for hiding this comment

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

missing an ending paren )

@@ -0,0 +1,77 @@
// +build !remoteclient
Copy link
Member

Choose a reason for hiding this comment

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

This means Podman has to use this too but libpod is using ABISupport. @baude, cool with that?

Copy link
Member

Choose a reason for hiding this comment

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

yeah i think it should probably be ABI support ... or be fenced by os !linux

Copy link
Member

Choose a reason for hiding this comment

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

!linux would be my favourite option.

Copy link
Member

Choose a reason for hiding this comment

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

decided to use remote

@baude
Copy link
Member

baude commented May 28, 2020

@rhatdan patch works .. please update and lets get merged.

Modify validate functions to work on a remote clients.
Any of the path checks will not work on remote machines or make
sense on remote clients. Therefore they should not be checked.

Signed-off-by: Daniel J Walsh <[email protected]>
@rhatdan rhatdan merged commit c292d84 into containers:master May 28, 2020
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.

5 participants