Join GitHub today
GitHub is home to over 40 million developers working together to host and review code, manage projects, and build software together.
Sign upstages/files: do not fail if links are correct #798
Merged
Conversation
This comment has been minimized.
This comment has been minimized.
|
Depends on #794 |
ajeddeloh
force-pushed the
ajeddeloh:link-state-sync
branch
4 times, most recently
from
276cba6
to
1809514
|
generally LGTM |
| @@ -45,6 +45,8 @@ func (s *stage) createFilesystemsEntries(config types.Config) error { | |||
|
|
|||
| // filesystemEntry represent a thing that knows how to create itself. | |||
| type filesystemEntry interface { | |||
| // create creates the entry if specified. It assumes that if overwrite=true then any existing | |||
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
ajeddeloh
Apr 23, 2019
Author
Contributor
That's the golang comment style. <functionname> <verbs> <stuff>
| return nil | ||
| } | ||
| } | ||
| return fmt.Errorf("error creating symlink %s: a non-symlink already exists that that path and overwrite is false", s.Path) |
This comment has been minimized.
This comment has been minimized.
| if st.Mode()&os.ModeSymlink != 0 { | ||
| if target, err := os.Readlink(s.Path); err != nil { | ||
| return fmt.Errorf("error reading link at %s: %v", s.Path, err) | ||
| } else if filepath.Clean(target) != s.Target { |
This comment has been minimized.
This comment has been minimized.
jlebon
Apr 23, 2019
Member
Sanity-check: is s.Target also already cleaned? Or should this be filepath.Clean(target) != filepath.Clean(s.Target)?
This comment has been minimized.
This comment has been minimized.
ajeddeloh
Apr 23, 2019
Author
Contributor
Thought it was, turns out it wasn't. Adding it here (and will PR with it to require clean paths for target).
doc/configuration-v3_0.md
Outdated
| @@ -81,7 +81,7 @@ The Ignition configuration is a JSON document conforming to the following specif | |||
| * **_name_** (string): the group name of the owner. | |||
| * **_links_** (list of objects): the list of links to be created. Every file, directory, and link must have a unique `path`. | |||
| * **path** (string): the absolute path to the link | |||
| * **_overwrite_** (boolean): whether to delete preexisting nodes at the path. Defaults to false. | |||
| * **_overwrite_** (boolean): whether to delete preexisting nodes at the path. Defaults to false. If overwrite is false and a matching link exists at the path, Ignition will only set the owner and group. | |||
This comment has been minimized.
This comment has been minimized.
jlebon
Apr 23, 2019
Member
Minor: in files and directories, we finish the blurb with Defaults to false. Should probably be consistent here too.
This comment has been minimized.
This comment has been minimized.
|
Updated with fixes, except the go comment thing because that's the way golang wants it. |
doc/configuration-v3_0.md
Outdated
| @@ -81,7 +81,7 @@ The Ignition configuration is a JSON document conforming to the following specif | |||
| * **_name_** (string): the group name of the owner. | |||
| * **_links_** (list of objects): the list of links to be created. Every file, directory, and link must have a unique `path`. | |||
| * **path** (string): the absolute path to the link | |||
| * **_overwrite_** (boolean): whether to delete preexisting nodes at the path. Defaults to false. | |||
| * **_overwrite_** (boolean): whether to delete preexisting nodes at the path. If overwrite is false and a matching link exists at the path, Ignition will only set the owner and group. Defaults to false | |||
This comment has been minimized.
This comment has been minimized.
If a link already exists that matches the specified link, do not fail. Also refactor links to use u.SetPermissions and fix u.SetPermissions to Lchown instead of Chown.
Add docs stating that Ignition will not fail (and instead just set user/group) when overwrite is false and Ignition encounters a matching link.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
ajeddeloh commentedApr 15, 2019
If a link already exists that matches the specified link, do not fail.
WIP needs tests, docs