serverless-toolkit icon indicating copy to clipboard operation
serverless-toolkit copied to clipboard

Unexpected behavior specifying project in .twilioserverlessrc using API keys

Open NReilingh opened this issue 4 years ago • 5 comments

I am migrating my projects from the Twilio Serverless Toolkit, twilio-run 2.x to 3.x, following this guide. Specifically, I noted the projects key described in the CONFIGURATION documentation as a possible solution for CI deployments.

I deploy to one Twilio Account from a staging branch and to another Twilio Account from a master branch, and CI is set up with different environment variables for those two deployment targets. These environment variables include the target Account SID, and an API Key/Secret pair for authentication.

In my CI job, I am calling

        npx twilio-run deploy
        --runtime node12
        --username ${TWILIO_API_KEY}
        --password ${TWILIO_API_SECRET}
        --environment pre-production

followed by

        npx twilio-run promote
        --username ${TWILIO_API_KEY}
        --password ${TWILIO_API_SECRET}
        --source-environment pre-production
        --production

I also have createEnvironment and overrideExistingProject set to true in .twilioserverlessrc. This works fine for the deploy step, but the promote step fails because it apparently needs a serviceSid specified. (Why?)

I don't like to have these types of values hardcoded in my repository, but since .twilioserverlessrc's projects key is designed to have configuration for multiple environments, I figured it would be acceptable to associate the serviceSid with the Account SID for each environment. I did so, but the promote action still failed. Why? Because the $TWILIO_API_KEY was being treated as the account identifier for referencing values from projects. When I changed the projects keys to use the CI environment's API keys instead of the Account SIDs, the promote command from my CI environment was successful.

Frankly, I don't think any of this should be necessary -- the serverless toolkit should be able to understand its authorization context after authenticating with provided API keys. Or at least pay attention when a TWILIO_ACCOUNT_SID (or ACCOUNT_SID) environment variable exists in the execution environment OR in the local .env file. Either way, I'm sure the intent of projects is to have values associated with ACxxxxxxxxxxxxxxxxxxxxxx keys and not SKxxxxxxxxxxxxxxxxxxxxxx keys, since the former is effectively static and the latter is highly context-sensitive.

NReilingh avatar Jul 04 '21 01:07 NReilingh

Hey @NReilingh! Yes you hit the combination of a couple of things here.

Quick clarification question: Are you running the two commands directly after each other or is there a validation step in between? If so what is the intention?

To address your questions:

  1. Why does the promote command require a serviceSid? The intention was to avoid messing around accidentally with the wrong command. In theory you don't have to pass one if you execute the promote command on the same system as the deploy command since we store it in the .twiliodeployinfo but that might have broken during the migration for the twilio-run use case so I'll investigate that.
  2. Why isn't projects working as intended? From a technical perspective we don't need the Account SID if you use API Key & Secret to do the deployment which is why we don't require it for twilio-run. However, if you use the functionality through the Twilio CLI and plugin-serverless we always have the Account SID and can do the lookup. The projects scoping was very much developed for scenarios such as using multiple Twilio accounts as outlined in this docs guide. I agree it would be nice to provide the same functionality for twilio-run and I'll look into it.
  3. Why is Account SID ignored?. This is semi-related to (2) but there's also the caveat that when we parse the .env file we actually ignore the ACCOUNT_SID and AUTH_TOKEN values for anything but local development since during the deployment Twilio Functions will expose those automatically to the respective service based on whatever account it's deployed to.

To unblock you in the meantime your best option is probably to have two separate config files that you can provide switch using the -c [path to config] flag.

dkundel avatar Jul 06 '21 20:07 dkundel

Hi @dkundel!

Yes, I am indeed running these commands one directly after another. If I recall correctly, this was not my original intention, but I found that there was no other way to deploy to the production environment directly using the deploy command -- if I specified "production" as the environment, it would deploy to a non-production environment named "production", meaning the function URL would still have the environment name as a suffix of the subdomain. I could only deploy to the true prod subdomain with no environment suffix if I used promote. If that has changed since when I originally wrote this workflow, I might be able to rewrite to avoid the second step.

  1. I don't commit .twiliodeployinfo to my repo, but if I understand you correctly, deploy should create it in the CI environment and then promote should read from it to identify the service SID, yes? I don't know for sure if my CI environment (Bitbucket Pipelines) is blowing away the .twiliodeployinfo file between the two steps, but it would be a good idea for me to retain that file as a build artifact which should also retain it for the promote step. I will make that change to my CI script and verify if promote is able to read it or is ignoring it.

  2. That's interesting that plugin-serverless and twilio-run are meant for slightly different use cases. I've basically been considering them to be equivalent, excepting that twilio-run is more accessible to an npm project. I'll read up on the new material in those docs -- I'm sure there's been updates since I last read them.

Thinking about how I would integrate the -c config file pattern with CI -- I'm familiar with a pattern commonly used with google cloud platform tools in CI, which is to store a base64-encoded JSON file in an environment variable, and then decode it out to a file on disk as the first step in the CI script. So I think what I might do is remove .twilioserverlessrc from the repo, inject it into the build environment as the first step in the CI script, and then twilio-run should pick it up automatically without even needing the -c option.

NReilingh avatar Jul 06 '21 21:07 NReilingh

Some quick followups:

  • I think I was just heeding the advice in twilio-run deploy --help regarding the --production option that says "please prefer the "activate" (i.e. promote) command". So it's not a limitation, but does seem to be anti-recommended by the docs (though now in hindsight I would assume that's more for safety reasons than as a deprecation warning.)

  • It turns out I already was trying to retain .twiliodeployinfo in my CI environment, and I confirmed locally that twilio-run promote is not reading the Service SID from .twiliodeployinfo if it exists.

  • I am reminded that .twilioserverlessrc has a lot of other options in it, so injecting just the project SID isn't a great idea unless I also merge the inject in with the config that is already there. I think a more elegant solution might just be to fetch the serviceSid from the generated .twiliodeployinfo file and pass it directly in to twilio-run promote as a command-line argument.

NReilingh avatar Jul 06 '21 21:07 NReilingh

To summarize my previous messages with some new analysis, I've figured out a couple of issues with .twiliodeployinfo that may need attention:

  • When authenticating twilio-run deploy using an API key instead of an Account SID and auth token, .twiliodeployinfo is not created.
  • When twilio-run promote is authenticated using an API key, it does not acquire the serviceSid contextually, nor does it read it from the .twiliodeployinfo file if it exists.

NReilingh avatar Jul 07 '21 16:07 NReilingh

Thanks for your investigation @NReilingh! I'll look further into how we can get around this issue. And yes your interpretation is right you can use --production with twilio-run deploy it's not a deprecation notice as much as it was intended to suggest to people to use multiple environments and promote between them rather than to deploy directly to multiple environments since those would technically be different builds but the messsage can be read as a deprecation notice so I'll rethink the wording here.

dkundel avatar Jul 07 '21 19:07 dkundel