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

Ignore broken symlinks and outside path, in commit #142

Merged
merged 1 commit into from
Apr 6, 2021

Conversation

squaremo
Copy link
Member

@squaremo squaremo commented Apr 2, 2021

There is a bug in go-git which leads to it reporting broken, absolute
symlinks as modified whether they are or not:

https:/go-git/go-git/issues/253

To date, the controller checks whether the repo it has run an update
on is Clean, and as a consequence will run into the bug above if a
broken symlink is in the repo. The result is that it makes and pushes
an empty commit every interval.

To work around the problem, this commit adds a more careful check of
the repo status. Each file reported as modified is validated, firstly
by checking that it's under the path given; and secondly by checking
specifically that it's not a broken symlink.

For convenience, I have moved a few procedures around so they can be
used more readily by go tests.

Fixes #135.


firstly by checking that it's under the path given

Of this bit I am not convinced. It's possible you have a valid symlink under the path, targeting a file not under the path, and you want the update to work. It's also possible you've come to this arrangement by accident -- but I shouldn't presume so!

UPDATE: I removed the path limitation in another commit; it can be squashed in, or reset out.

Copy link
Member

@stefanprodan stefanprodan left a comment

Choose a reason for hiding this comment

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

LGTM

Thanks @squaremo 🥇

@hiddeco please incorporate the symlink check in fluxcd/flux2#968

@squaremo
Copy link
Member Author

squaremo commented Apr 5, 2021

I'm going to squash in the commit removing the path limitation, then merge.

There is a bug in go-git which leads to it reporting broken, absolute
symlinks as modified whether they are or not:

    go-git/go-git#253

To date, the controller checks whether the repo it has run an update
on is Clean, and as a consequence will run into the bug above if a
broken symlink is in the repo. The result is that it makes and pushes
an empty commit every interval.

To work around the problem, this commit adds a more careful check of
the repo status. Each file reported as modified is validated by
checking specifically that it's not a broken symlink: if `os.Lstat`
says it's a symlink and `os.Stat` reports the (target) file is
missing, it can be ignored. (Why not just ignore any missing file?
Because a missing file might indicate some other problem, so better to
let it fail).

For convenience, I have moved a few procedures around so they can be
used more readily by go tests.

Signed-off-by: Michael Bridgen <[email protected]>
@squaremo squaremo merged commit f6ad224 into main Apr 6, 2021
@squaremo squaremo deleted the narrow-commit-scope branch April 6, 2021 09:03
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.

Empty commits
2 participants