new(redis-cli): Add support for Redis CLI with password as environment variable and command-line flags

[arunsathiya]

Overview

This PR adds support for redis-cli. The password for connecting to the redis server can be set as an environment variable and it'll be used by the shell plugin.

Type of change

• [x] Created a new plugin
• [ ] Improved an existing plugin
• [ ] Fixed a bug in an existing plugin
• [ ] Improved contributor utilities or experience

Related Issue(s)

• Resolves: https://github.com/1Password/shell-plugins/issues/275

How To Test

• Sign up for a Redis hosting provider like Upstash.
• Create a Redis server.
• Make note of the password, hostname and port number shown by that provider.
• Clone this branch and run make redis/build
• Set up redis shell plugin by running op plugin init redis-cli. Set the password as a 1Password field item.
• Run redis-cli -h host-address-here.com -p port-number-here PING
• See response PONG

Changelog

Add support for Redis CLI with environment variable-based provisioning.

[arunsathiya]

A couple of quick notes:

• ManagementURL is not set for the credential because, depending on the redis hosting provider the link will be different.
• We provision using environment variables for now, because REDISCLI_AUTH is the only support variable, for the password. There isn't one for the host, port or the database.
• There isn't a config file provisioner for redis CLI. So, that's not an option.
• I was exploring a new arguments provisioner for tunnelto, but it's on hold at this time. I can take another stab at it soon and when it's ready, we can adopt the same concept here for redis, so that host, port and database can be set up as individual field items too.

Let me know what you think about this!

[hculea]

Any reason not to use, in conjunction with the envvar provisioner, a command line provisioner (i.e. adding the port and host flags within the provisioner itself). The entire provisioner should be the equivalent of:

REDISCLI_AUTH=<value> redis-cli -p <port> -h <host>

[arunsathiya]

@hculea, that's a great suggestion, thank you!

So, today's the first time I properly dived into the provisioner SDK and I see that it's possible to provision two types of credentials (environment variables and command-line flags, in this case) in a single provisioner. The current shell plugin supports this behavior with a new AddArgsImmediatelyAfterExecutableName provisioner function.

Examples to test with:

• redis-cli ping actually translates into redis-cli -h host.com -p 1234 ping behind the scenes, and outputs pong.
• redis-cli incr counter_value translates into redis-cli -h host.com -p 1234 incr counter_value behind the scenes, and outputs (integer) 1

While Redis CLI supports multiple command line flags, I have worked here with the assumption that host and port are minimum-required flags. If the user supplies additional flags, they'll be used properly.

Example:

• redis-cli -r 5 INCR counter_value translates into redis-cli -h host.com -p 1234 -r 5 INCR counter_value behind the scenes, and outputs the following:

(integer) 1
(integer) 2
(integer) 3
(integer) 4
(integer) 5

Also, if the user decides to use a custom value for the host and port, as opposed to the value we have on the 1Password item, that shouldn't be a problem either. The flag value entered by the user will be used and the one from the 1Password item be skipped.

Let me know what you think of this setup!

[arunsathiya]

The current shell plugin supports this behavior with a new AddArgsImmediatelyAfterExecutableName provisioner function.

For some additional context on why this new function is needed, AddArgs function appends the command-line arguments to the end of the user-inputted command. That is, redis-cli incr counter_value becomes redis-cli incr counter_value -h host.com -p 1234 but this is not supported by Redis CLI for some reason. It results in the following error:

AUTH failed: ERR AUTH <password> called without any password configured for the default user. Are you sure your configuration is correct?
(error) ERR wrong number of arguments for 'incr' command

Redis CLI expects the flags to be immediately after the executable name, thus the new function AddArgsImmediatelyAfterExecutableName. So, the same command will translate to redis-cli -h host.com -p 1234 incr counter_value and output the correct response.

[arunsathiya]

Couple of additional notes for all reviewers:

• 👍🏼 Since the last iteration, we now have username as a separate field, thus allowing scoped/ACL-based user accounts.
• 👍🏼 There isn't a way to set the "default" value for the username at this time: https://github.com/1Password/shell-plugins/issues/281 I am hoping redis users will be familiar with setting the standard "default" username as the default, when they don't have ACL-based accounts.
• 👍🏼 Tested with Upstash, Redis.com (enterprise Redis cloud, from the same makers of open source Redis software), works well.
• 👎🏼 Yet to test on AWS Elasticache. More significantly, yet to test/understand how AWS-hosted redis clusters work, because based on a quick glance, their access is locked down to EC2 instances.

For the near future:

• Investigate and fix optional setting, so that it can be used on the username field: https://github.com/1Password/shell-plugins/issues/280
• Decide on how we should approach redis-Terraform compatibility. I talked about this with @AndyTitu a bit already. The main blocker is that, for our Terraform shell plugin, we'll need to use environment variables instead of command-line arguments for authentication. But the environment variables for various redis providers are different:

Name	Provider	Link	Environment variables
rediscloud	redislabs	Cell	REDISCLOUD_URL, REDISCLOUD_ACCESS_KEY and REDISCLOUD_SECRET_KEY
redisdb	CruGlobal	Cell	REDISDB_DATABASE, REDISDB_HOSTNAME, REDISDB_PORT
rediscloud	DevopsHomeserve	Cell	REDISCLOUD_URL, REDISCLOUD_ACCESS_KEY and REDISCLOUD_SECRET_KEY

As we can see here, these environment variables vary between providers, and most importantly, vary from the open source redis software, which offers only REDISCLI_AUTH.

So, we will need to think of how to best-approach Terraform compatibility. In the meantime, I propose we push forward the current version, as there's significant value for Upstash and redis.com cloud users.

[AndyTitu]

This is what Arun and I discussed earlier today.

The approach is to use 2 different credential usages (for 2 different credentials), 1 for cloud and 1 for local, and to provision them using the above strategies. In this way, we enable the Redis cloud + terraform use case, and we also enable redis-cli authentication both via ACL and "legacy" password depending on whether the user specifies the optional username or not.

Here is each credential's structure:

[accraw]

I did a functional test with local docker + ACL and it worked beautifully.

[arunsathiya]

Updated thought process:

• REDISCLOUD_ACCESS_KEY and REDISCLOUD_SECRET_KEY environment variables from the rediscloud Terraform provider are not for connecting to a redis server hosted on Redis.com, as I had originally understood. They are for connecting to the redis.com enterprise platform itself, to create and manage servers, amidst various other features.
• It makes sense to support redis.com authentication on the same shell plugin instead of creating an entirely new shell plugin for it, even though it's only used with the Terraform provider and not on a CLI binary.

So, the updated credentials are as follows:

• User Credentials which is a combination of username, password, host and port. Only password is mandatory. Default username is "default", default host and port are 127.0.0.1 and 6379. If someone wants to connect to a cloud redis server, they'd obviously need to define the host and port.
• Secret Key which is a combination of Access Key and Secret Key fields, which are obtained from redis.com's API keys page, and facilitate connection to the redis.com cloud platform.

New testing instructions, which can be tested only by internal contributors:

• Fetch the internal CLI changes and run op plugin init redis-cli
• When prompted to choose one of the two credentials to set up, start with User Credentials. Configure username (either "default" or a custom user account created using ACLs), password, host and port.
• Make sure that redis-cli commands (some commands to run here) respond as expected.
• Run op plugin init redis-cli again and this time, choose "Secret Key" credential and save that credential to your 1Password work.
• Create main.tf in a folder and run the following commands in order: terraform init > terraform plan > terraform apply. All of these commands use the Secret Key credential created in the last step.

Sample main.tf file to test with:

terraform {
  required_providers {
    rediscloud = {
      source = "RedisLabs/rediscloud"
      version = "1.1.1"
    }
  }
}

provider "rediscloud" {}

data "rediscloud_regions" "example" {
}

output "all_regions" {
  value = data.rediscloud_regions.example.regions
}

[arunsathiya]

Those failing validation checks are tracked internally now. I think we'll need to discuss and conclude how we handle those as well before we can merge this PR.

[arunsathiya]

@AndyTitu, thanks for all the feedback! I trust all changes have been addressed but let me know if you spot anything off.

I have also done a functional testing at the latest commit, https://github.com/1Password/shell-plugins/pull/276/commits/92af35eddb85552fd5aa3eafb3060c9fba4250dd, and all cases work well.

[arunsathiya]

Regarding the merge conflict, I am going to rebase with main once the reviews are done. I could do that right now, but I have a feeling that pushing after the rebase will change the commit IDs? 🤔 So, I'll hold off now.

[AndyTitu]

Those failing validation checks are tracked internally now. I think we'll need to discuss and conclude how we handle those as well before we can merge this PR.

This is the failing check: ✘ Has no more than one credential type defined. Plugins with multiple credential types are not supported yet

We conceptually no longer need this. I have also confirmed that we don't need it from a technical point of view by testing the latest version of redis plugin with the latest cli.

@arunsathiya feel free to remove it.

[arunsathiya]

@AndyTitu Appreciate the feedback!

I built AddArgsAtIndex(1, argsMap) as suggested, but there's a sticky point with it. The only two valid indices would be: 1 and len(args), where 1 corresponds to immediately after the executable name and len(args) corresponds to the end of the command line. If one were to set a different value, including 0, provisioning breaks. This especially gets complicated when CLI programs stress importance on the order of flags.

One example is, redis handles redis-cli -h value -p value incr key and redis-cli incr hey -h value -p value differently. And if I were to do something like, AddArgsAtIndex(2, argsMap), that results in redis-cli incr -h value -p value key, which breaks entirely.

So, in https://github.com/1Password/shell-plugins/pull/276/commits/fe969f8134371df21450f78d0c27235ebd3f19b6, I have taken a different approach: AddArgs(provisionImmediatelyAfterExecutable bool, args ...string). The value set for provisionImmediatelyAfterExecutable controls where the provisioned arguments go.

Let me know what you think of this!

[arunsathiya]

Thanks for the review/feedback, @jpcoenen! All suggestions have been addressed now.

[arunsathiya]

Would it be possible to completely decouple this from that other PR by using a custom provisioner for Redis for now that sets both the environment variable and the arguments? Or would that add a lot of overhead?

@jpcoenen I chatted about it with @AndyTitu too, and we agree on decoupling this from the other PR with SDK-level arguments provisioner. So, I have removed it from the redis work and introduced a custom redis provisioner for now. I trust this is ready for one final review now.

[arunsathiya]

Been a while since I last worked on this. Some progress notes and testing instructions below!

New changes:

• Changes made in https://github.com/1Password/shell-plugins/pull/276/commits/47d5292eb521fc6b9d8ea544393e8991950673fc ensure that secrets are sourced from 1Password, but if an user provisions them manually as command line arguments, they are prioritized.
• Authentication is not skipped by using these arguments.

Testing instructions:

• Set up redis shell plugin with op plugin init redis-cli
• Import host, port, username and password as part of the setup flow
• Run a test command like redis-cli incr key. What happens behind the scenes is redis-cli -h <value> -p <value> --user <value> incr key (redis CLI requires that these flags are immediately after redis-cli and before the executing command. That's why we provision these flags at index 1), and the password is provisioned as a REDISCLI_AUTH environment variable.
• Run a test command like redis-cli incr key -p 19261 or redis-cli -p 19261 incr key. These cases should be handled as well. Behind the scenes, the provisioner does redis-cli -h <value> -p 19261 --user <value> incr key and REDISCLI_AUTH for password.