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

Add support for Fastly with envvar provisioning and config file imports #169

Merged
merged 2 commits into from Feb 15, 2023

Conversation

arunsathiya
Copy link
Contributor

@arunsathiya arunsathiya commented Jan 31, 2023

This PR adds support for Fastly CLI. Fixes #133.

Key things to know:

  • Fastly uses API Tokens to authenticate.
  • FASTLY_API_TOKEN envvar (environment variable) is the easiest way to authenticate. The CLI also supports -t, --token as command flags to login, but environment variables are preferred.

Issues that need to be discussed

Issue with profile subcommands

fastly profile command has a bunch of sub-commands create, delete, list, switch, token and update. Of these commands, except for list subcommand, all subcommands can be used in the form of the same environment variable (I think), but that's not possible just yet because Shell Plugins does not offer a simple way to switch to another credential:

Until that feature is available, I am suggesting that we opt out of 1Password authentication for fastly profile commands.

The other command, fastly profile list basically looks for the list of available profiles in the config file and operate on the data available there. With Shell Plugins though, the config file will basically be void of these profiles, which in turn means that that command will just fail.

Overall, to me, it makes sense to avoid 1Password authentication for fastly profile commands for now.

Issue with config file imports

For some reason, I am unable to get MacOnly and LinuxOnly OS-specific config file imports working. It's probably something simple that I am missing. cc @hculea as we previously scheduled a call to discuss this. 🙏🏼 Thank you for any insights!

shyim
shyim previously approved these changes Jan 31, 2023
Copy link
Contributor

@shyim shyim left a comment

Choose a reason for hiding this comment

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

wow thanks ❤️ . Looks really good!

@Integralist
Copy link

👋🏻 @arunsathiya

If there are any issues with the Fastly CLI that you think might make integration with 1Password easier, then do please open an issue at https://github.com/fastly/cli

@arunsathiya arunsathiya marked this pull request as draft February 3, 2023 17:37
@arunsathiya
Copy link
Contributor Author

Converting to draft for now because I need help with OS-specific importers.

@hculea
Copy link
Member

hculea commented Feb 6, 2023

Thank you for this addition, Arun! ❤️ I've looked over the importers and both look good to me. The paths also look to be correct - user config dir suffixed by fastly/config.toml.

Off the top of my head, I could see a possible problem on UNIX systems (aside from MacOS), in the situation where the user would have the XDG_CONFIG_HOME envvar pointing to a custom location.

Is it possible that this is the case, in your situation? If not, could you maybe describe what is the unexpected behaviour that you're encountering, when trying to run the file importers? Thanks!

@arunsathiya
Copy link
Contributor Author

Thanks for taking a look, @hculea. XDG_CONFIG_HOME is empty for me:

➜  ~ echo $XDG_CONFIG_HOME

However, I have a positive news. Mac OS config file import is working as expected now. I am not sure what I was doing incorrectly the last week. Perhaps I didn't have any configuration profiles on the /Users/arun/Library/Application\ Support/fastly/config.toml file. 🤷🏼

I tried testing it on Linux (Ubuntu) on Docker, but it seems that Shell Plugins aren't supported on it:

root@c4fc0e1feba7:~/.config/op/plugins/local# ls
fastly
root@c4fc0e1feba7:~/.config/op/plugins/local# cd ~
root@c4fc0e1feba7:~# op plugin init fastly
WARNING:  is not a shell officially supported by 1Password Shell Plugins!
[ERROR] 2023/02/08 04:34:49 unknown plugin: fastly

I am not too sure how to move forward in terms of Linux testing, but given that the path is correct, we could assume that it works fine on Linux.

At this point in time, this PR is ready for a review! 😄

@arunsathiya arunsathiya marked this pull request as ready for review February 8, 2023 04:39
@arunsathiya
Copy link
Contributor Author

Side note:

I generated the fastly shell plugin build on my Mac and copied to the Docker container using docker cp ~/.op/plugins/local/fastly c4fc0e1feba7:/root/.config/op/plugins/local, but I don't know if that's a sufficient way to do it or if the build for Linux will completely vary.

…it is only used to print out the config file or it's location
@AndyTitu
Copy link
Contributor

AndyTitu commented Feb 10, 2023

I think you should copy the config to the .op file location. This is the only location used by locally built shell plugins: docker cp ~/.op/plugins/local/fastly c4fc0e1feba7:/root/.op/plugins/local @arunsathiya This should solve the above issue.

@arunsathiya
Copy link
Contributor Author

@AndyTitu, I am unable to copy to that location because that location doesn't exist:

➜  ~ docker cp ~/.op/plugins/local/fastly a1ac133d0753:/root/.op/plugins/local
Error: No such container:path: a1ac133d0753:/root/.op/plugins

Following this documentation, the envvar XDG_CONFIG_HOME isn't set in my Docker-based Ubuntu container, which is why I am copying to /root/.config/op/plugins/local

root@a1ac133d0753:/# echo $XDG_CONFIG_HOME

Overall though, I believe we don't have any blockers, except that I have not tested the Linux importer.

Copy link
Member

@hculea hculea left a comment

Choose a reason for hiding this comment

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

Looks good to me! Thank you for this contribution! ❤️

@hculea
Copy link
Member

hculea commented Feb 15, 2023

I think the issue in your tests above @arunsathiya is that the path actually doesn't exist, since those directories never got created (that happens automatically, when building a plugin). Running a mkdir -p /root/.config/op/plugins/local inside the container should fix the issue.

One more thing to have in mind - the binaries are OS dependent, so you may want to set GOOS and GOARCH to those of your container, before running the make build command on the plugin.

That being said, I don't see not having tested the Linux importer as a blocker. Good to go IMO! 🚢

Copy link
Member

@SimonBarendse SimonBarendse left a comment

Choose a reason for hiding this comment

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

Regarding the fastly profile commands, I think it makes total sense for the 1Password Shell Plugin not to operate on those. When you're using the 1Password shell plugin for Fastly, you no longer need to use these commands, since you're managing the profiles/credentials through 1Password. So this is perfect as is 👍 🚀 We'll include it in the next 1Password CLI release.

Appreciate yet another amazing contribution, thank you @arunsathiya ! ❤️

@SimonBarendse SimonBarendse merged commit 05912c0 into 1Password:main Feb 15, 2023
@arunsathiya arunsathiya deleted the add/fastly branch February 15, 2023 17:26
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.

Fastly cli
6 participants