* Safer handling of merging with an updated upstream.
We used to call forceClone() to update with upstream, but this deletes the
checked out directory. This is inefficient, can delete existing plan files,
and is very surprising if you are working manually in the working directory.
We now fetch an updated upstream, and re-do the merge operation. This
leaves any working files intact.
* Rename SafeToReClone -> CheckForUpstreamChanges
It's never safe to clone again. But sometimes we need to check
for upstream changes to avoid reverting changes.
The flag is now used to know when we need to merge
again non-destructively with new changes.
* Update fixtures.go
* Add test to make sure plans are not wiped out
As long as the branch itself has not been updated, plans
should be kept. Even if upstream has changed.
* renamed HasDiverged to MergedAgain in PlanResult and from Clone()
This flag was only set to true in case a call to Clone()
ended up merging with an updated upstream, so the
new name better represents what it means.
* Test that Clone on branch update wipes old plans
This complements the test that Clone with unmodified
branch but modified upstream does _not_ wipe plans.
* runGit now runs git instead of returning a function that runs git
* Updated template to merged again instead of diverged
This is no longer a warning, but expected behavior in merge chekout mode
* Rename git wrapper to wrappedGit, add a type for static config
Every call to wrappedGit for the same PR uses identical setup
for directory, head repo and PR, so passing the
---------
Co-authored-by: nitrocode <7775707+nitrocode@users.noreply.github.com>
Co-authored-by: PePe Amengual <jose.amengual@gmail.com>
* whitespace fixes
* Only re-clone on upstream updates just before planning
When using the merge checkout strategy, we would peviously
re-clone on upstream updates on any call to Clone(), for example
when running workflow hooks or runing import commands.
If we had already ran a plan, and master was updated, this
would delete the plan and apply would not have anything to
apply.
Modified to only re-clone on upstream updates in doPlan()
* Apply suggestions from code review
---------
Co-authored-by: PePe Amengual <jose.amengual@gmail.com>
Co-authored-by: nitrocode <7775707+nitrocode@users.noreply.github.com>
* merge again if base branch has been updated
The current "merge" checkout strategy is unsafe. It merges the PR and the base
branch without holding the directory lock(s), so there is a potentially
very long window where another PR can be applied and be unexpectedly reverted
later.
This happens occasionally if a PR causes plans in multiple directories, but
is almost _guaranteed_ to happen if the initial plan has to wait until a
lock is freed up and a manual "atlantis plan" command is given.
Instead of printing a warning when this happens, we now merge again
if necessary while holding the lock. Plans are then guaranteed to only
be made when merged with the base branch for each directory being planned,
and applying later should be safe even if the base branch sees further
updates.
This fixes/affects #804, #867, #979
* Remove diverged test that no longer applies
* Reinstate TestClone_MasterHasDiverged test and make it pass
* Extend TestClone_MasterHasDiverged to test new merging functionality
We now verify that the first Clone with CheckoutMerge=true with
a diverged base branch atually clones and merges again.
---------
Co-authored-by: PePe Amengual <jose.amengual@gmail.com>
This commit replaces `os.MkdirTemp` with `t.TempDir` in tests. The
directory created by `t.TempDir` is automatically removed when the test
and all its subtests complete.
Prior to this commit, temporary directory created using `os.MkdirTemp`
needs to be removed manually by calling `os.RemoveAll`, which is omitted
in some tests. The error handling boilerplate e.g.
defer func() {
if err := os.RemoveAll(dir); err != nil {
t.Fatal(err)
}
}
is also tedious, but `t.TempDir` handles this for us nicely.
Reference: https://pkg.go.dev/testing#T.TempDir
Signed-off-by: Eng Zer Jun <engzerjun@gmail.com>
Signed-off-by: Eng Zer Jun <engzerjun@gmail.com>
This fixes the tests to not rely in the implicit value of the
"defaultBranch" git configuration value, since they later assume that
it's "master".
For example, if you run:
```sh
git config --global init.defaultBranch main
```
And then run these tests, you get:
```sh
--- FAIL: TestClone_CheckoutMergeNoneExisting (0.68s)
working_dir_test.go:71: err running "git checkout master": error:
pathspec 'master' did not match any file(s) known to git
--- FAIL: TestClone_CheckoutMergeNoReclone (0.68s)
working_dir_test.go:127: err running "git checkout master":
error: pathspec 'master' did not match any file(s) known to git
--- FAIL: TestClone_CheckoutMergeNoRecloneFastForward (0.68s)
logger.go:130: 2022-05-17T10:05:33.665-0700 INFO creating
dir "/tmp/3762936464/repos/0/default"
working_dir_test.go:198: unexpected error: running git
clone --branch master --single-branch
file:///tmp/3657236513 /tmp/3762936464/repos/0/default:
Cloning into '/tmp/3762936464/repos/0/default'...
warning: Could not find remote branch master to
clone.
fatal: Remote branch master not found in
upstream origin
: exit status 128
--- FAIL:
TestClone_CheckoutMergeConflict
(0.68s)
working_dir_test.go:232: err running "git checkout master": error:
pathspec 'master' did not match any file(s) known to git
--- FAIL: TestClone_MasterHasDiverged (0.68s)
working_dir_test.go:344: err running "git clone --branch
master --single-branch /tmp/1931262087 .": Cloning into '.'...
warning: Could not find remote branch master to clone.
fatal: Remote branch master not found in
upstream origin
--- FAIL: TestHasDiverged_MasterHasDiverged
(0.68s)
working_dir_test.go:415: err running "git clone --branch master
--single-branch /tmp/2356723317 .": Cloning into '.'...
warning: Could not find remote branch master to clone.
fatal: Remote branch master not found in upstream
origin
FAIL
FAIL
github.com/runatlantis/atlantis/server/events
7.269s
FAIL
```
* fix: add path to WorkingDir methods
`WorkingDirLocker` and `WorkingDir` are closely related -- the former acquires a lock, which ensures that the latter can safely operate on a given file path.
In #2131 we added `path` to the `WorkingDirLocker` lock key, but neglected to add the same to `WorkingDir`.
This commit adds `path` as an argument to certain `WorkingDir` methods, and includes `path` in the directory that we use to clone the repository for a given project.
Since `path` can include certain special characters such as `/`, we encode `path` as base32 when using it as part of a file path. This should ensure no special characters are used in the filesystem, and that the value can be decoded if desired (unlike hashes such as md5).
Additional changes:
- All calls to changed methods have been updated, including unit tests
- Mocks have been regenerated for `WorkingDir` and `WorkingDirLocker`
- `working_dir_test.go`
- Commands that operate on filesystem paths have been updated to include the base32 encoded `path` value
- When running `git init`, we include `-b master` as additional arguments, to ensure that `master` is our default branch (expected by the unit tests)
* Try fixing E2E tests
* Fix DefaultPendingPlanFinder
In addition to iterating over `workspaceDirs`, also iterate over `pathDirs`, and use both `workspace` and `path` to build `repoDir`.
* Fix DefaultPendingPlanFinder unit tests
* Fix DefaultProjectCommandBuilder unit tests
Co-authored-by: Kevin Snyder <kevinsnyder@KevinSnydersMBP.lan>
Co-authored-by: Kevin Snyder <kevinsnyder@ip-192-168-1-188.ec2.internal>
* ORCA-731 Force PR authors to rebase if master has diverged (#70)
* initial commit for undiverge apply req
* Update documentation
* Fix existing unit tests
* HasDiverged unit test
* project command runner unit test
* Global config tests cases
* update parser unit tests
* Update project_test unit tests
* Fix linting
* fix test case with logging output
* add space on readme markdown
* Remove unused variable
* remove deprecated flag option
* Use global args struct instead of passing in args
* Fix merge issue
This is two changes:
Replacing all usages of SimpleLogger with it's interface SimpleLogging which makes it easier to swap out loggers going forward
Nukes SimpleLogger in favor of StructuredLogger which allows us to better analyze logs in our log management system we choose.
- rename gh-app-key to gh-app-key-file for clarity
- change git credentials writer to append a line if there is an existing
.git-credentials file and in the case of the github app to replace the
old github app line
- removed automatically setting --write-git-creds to true when using a
github app and instead requiring this is set specifically
When using --checkout-strategy merge, we were performing the merge
without the --no-ff flag. This meant that if the branch could be merged
via a fast-forward merge (without a merge commit), then later when we
ran git rev-parse HEAD^2 to determine if the repo was at the right
commit, it would fail.
Now we merge with --no-ff which ensures the commit tree will always look
the same and git rev-parse HEAD^2 will always succeed.
