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

fix: add path to WorkingDir methods #2180

Merged
merged 5 commits into from
Apr 8, 2022

Conversation

KevinSnyderCodes
Copy link
Contributor

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.

path is the directory for an Atlantis project, relative to the root of the repository.

This commit adds path as an argument to certain WorkingDir methods, and includes path in the file path that we use to clone the repository for a given Atlantis 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 that 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)

Kevin Snyder and others added 5 commits April 4, 2022 14:16
`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 runatlantis#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)
In addition to iterating over `workspaceDirs`, also iterate over `pathDirs`, and use both `workspace` and `path` to build `repoDir`.
Copy link
Contributor

@jamengual jamengual left a comment

Choose a reason for hiding this comment

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

LGTM

@jamengual
Copy link
Contributor

thanks for the contribution @KevinSnyderCodes

@jamengual
Copy link
Contributor

Please give it a try in the prerelease

@jamengual jamengual merged commit 1a83444 into runatlantis:master Apr 8, 2022
@edbighead
Copy link
Contributor

edbighead commented Apr 12, 2022

might be unrelated but I started to get following with the latest dev image

dir ".tmp/prod-production/my-service/infra" does not exist

I'll try using absolute paths for dir. If that works maybe worth mentioning in docs that it should be an absolute path

@jamengual
Copy link
Contributor

jamengual commented Apr 12, 2022 via email

@edbighead
Copy link
Contributor

edbighead commented Apr 12, 2022

@jamengual I did use FROM ghcr.io/runatlantis/atlantis:dev (Digest: sha256:4231a05fcecd16f942d396bbc3602926873e0dd5faee3320b38a835ca23736ee) which is an image built 1 day ago.

@jamengual
Copy link
Contributor

mmm I think dev and prerelease are the same, we should delete the dev tag but just in case this is the one:

docker pull ghcr.io/runatlantis/atlantis:v0.19.3-pre.20220408

@jamengual
Copy link
Contributor

@edbighead please report back your findings

@edbighead
Copy link
Contributor

@edbighead please report back your findings

sure, I will. gonna check the 0.19.2 and prerelease versions. We use pre_workflow_hooks and changed them as well, maybe this causes dir ".tmp/prod-production/my-service/infra" does not exist and not the version change

@edbighead
Copy link
Contributor

edbighead commented Apr 19, 2022

@jamengual @KevinSnyderCodes I do get the below mentioned error on ghcr.io/runatlantis/atlantis:v0.19.3-pre.20220408 and don't get it with ghcr.io/runatlantis/atlantis:v0.19.2

It happens in the repo with pre_workflow_hooks:

repos:
  - id: github.com/org/repo
    apply_requirements: ["approved"]
    allowed_overrides: [workflow]
    allow_custom_workflows: true
    pre_workflow_hooks:
      - run: render --path . --output .

where render script renders the atlantis.yaml in the cloned repository.

atlantis.yaml

version: 3
projects:
  - dir: .tmp/prod-production/my-service/infra
    name: prod-production-my-service-infra
    autoplan:
      when_modified: ["../../../../apps/my-service/app.tf"]
      enabled: true

Error:

dir ".tmp/prod-production/my-service/infra" does not exist

@jamengual
Copy link
Contributor

reverted due to : #2239

@snorlaX-sleeps snorlaX-sleeps mentioned this pull request Oct 7, 2022
7 tasks
krrrr38 pushed a commit to krrrr38/atlantis that referenced this pull request Dec 16, 2022
* 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 runatlantis#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>
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.

3 participants