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

Option to traverse up the directory hierarchy looking for secrets.yml #122

Closed
marco-m opened this issue Jan 25, 2020 · 7 comments · Fixed by #144
Closed

Option to traverse up the directory hierarchy looking for secrets.yml #122

marco-m opened this issue Jan 25, 2020 · 7 comments · Fixed by #144

Comments

@marco-m
Copy link
Contributor

marco-m commented Jan 25, 2020

Imagine having a project with a top-level secrets.yml file and being deep inside the directory hierarchy when using summon. Imagine also, like in a terraform project, that there are multiple directories from where it makes sense to invoke summon to have access to secrets.

For example: project layout

$ pwd
~/myproject
$ tree
.
├── secrets.yml
└── tf
    ├── dev
    │   ├── user-alice
    │   │   └── main.tf
    ├── prod
       └── ...

If I am doing work in directory tf/dev/user-alice, the invocation will be

$ pwd
~/myproject/tf/dev/user-alice
$ summon -f ../../../secrets.yml terraform plan

If I change directory level, I will have to keep adjusting the relative ../.. path to locate secrets.yml.

To avoid all this, the proposal is to add a new option -u to traverse up the directory hierarchy, until it finds the first secrets.yml:

$ pwd
~/myproject/tf/dev/user-alice
$ summon -u terraform plan

in this case, summon will go up 3 times until it finds ~/myproject/secrets.yml :-)

If this proposal is accepted, I will be happy to provide a PR.

I took inspiration for this behavior from the build tool scons; the -u option is explained in the scons manual page.

@BradleyBoutcher
Copy link
Contributor

Hi Marco! Thank you for reaching out with your suggestion. We're always looking for ways to improve the user experience and this is a solid idea.

For this use case, we think the best solution would be as follows:

  • -f remains optional, and can take a file name or filepath, defaulting to secrets.yml
  • -r directs summon to search recursively in child directories
  • -u directs summon to search recursively through the parent directories

The last two flags should be mutually exclusive, and could possibly be implemented using filepath.Walk in golang.

If you intend to implement one of these, your commits should include the --signoff flag (https://git-scm.com/docs/git-commit#Documentation/git-commit.txt--s) to certify that you have the rights to submit the work under the same license as the project and you agree to a Developer Certificate of Origin (see http://developercertificate.org/ for more information).

@BradleyBoutcher
Copy link
Contributor

BradleyBoutcher commented Jan 27, 2020

I went ahead and worked on this a little, and have a mostly finished change on
this branch.

The only thing it needs is a check for reaching the end of the search to avoid infinite looping,
and the documentation for command line prompts.

Git Diff here

@marco-m
Copy link
Contributor Author

marco-m commented Jan 27, 2020

Hello Bradley,
happy that you like the proposal. I have some comments and questions.

  • agree on -f. Said in another way, -f behavior doesn't change. And is incompatible with -u.
  • I am not sure on -r, because, unlike -u, its behavior is non-intuitive: what if there are multiple child secret.yml down the directory tree? Which one will be picked? Sure, one could document it as "following the same logic as filepath.Walk", or "breadth-first search" but it is not very friendly to the user and in my opinion will be source of confusion. Also, it is independent from -u, and I would like to concentrate on -u.

EDIT

Ahah, you were faster than me :-)

@marco-m
Copy link
Contributor Author

marco-m commented Jan 30, 2020

Hello @BradleyBoutcher, for my understanding, how do you see the next steps?

@BradleyBoutcher
Copy link
Contributor

Hi @marco-m, sorry I didn't see this comment sooner!

Per your comment, I'm more than happy deferring to you for implementing the feature to meet your needs, and can review it as needed. You're welcome to pick up my branch or work on things independently, whatever you see fit. For the design, I do think that -f and -u should be compatible though, since a user may want to search up in a directory for a non-default file name.

@marco-m
Copy link
Contributor Author

marco-m commented Jun 25, 2020

Released! https://github.com/cyberark/summon/releases/tag/v0.8.2
thanks!

@izgeri
Copy link
Contributor

izgeri commented Jun 25, 2020

@marco-m sorry this took us a bit to release - in the future, feel free to log an issue requesting a release if you need one sooner. that will help us to get it scheduled. thanks again for adding this! it's a really nice addition to the tool :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

3 participants