awslabs / amplify-video

An open source Category Plugin for the AWS Amplify-CLI that makes it easy to deploy live and file based streaming video services and integrate them into your Amplify applications.
https://www.npmjs.com/package/amplify-category-video
Apache License 2.0
267 stars 56 forks source link

Support multiple backends & minor fixes #255

Closed armenr closed 3 years ago

armenr commented 3 years ago

Related Issue

Closes https://github.com/awslabs/amplify-video/issues/249

Related Conversation(s)

https://github.com/armenr/amplify-video/pull/1

Description of changes

Breaking changes

Pending PR code that's in this branch

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

armenr commented 3 years ago

@wizage - currently working through a minor issue we've hit during testing (uncertain about the difference in behavior between @arturocanalda 's results and mine...my best guess is we're using different versions of the amplify libs).

Actively working to resolve...

Additionally, I wasn't sure whether to add my code to build.js or video-staging.js...or both. Please feel free to call out anything I've missed, or any duplicative logic I should remove. Thank you! :)

arturocanalda commented 3 years ago

@armenr

As mentioned somewhere in another comment, I was using nodeJS 12.x, which I just upgraded and re-tested.

Current versions:

OS: macOS v10.14.6
Node: v14.16.1
Amplify: v4.50.2

Steps of my test:

For some reason Cloudformation doesn't accept that publicKey. But it does accept this one (my dev key that was created with amplify-video 3.1.0:

"-----BEGIN PUBLIC KEY-----\\nMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAuF9uG+JnbtDe9sGT+m1+\\nNCiIMtS0w6O7ajkJCX3jTfYv4fHTlOFN/VW+aPSbdevucU7I4BdIB4YPBumTDYxK\\nJXawTUm2ZVQUDOFXUMI0S5etoQKFZNcMFkXLjw0r8jjc4Hav6BZ+CFc5pq2djgJM\\nKQwbM6u3bJZNo5RBpNGNVjkd7JUKxuW77w1woBjX+yRdNRucFhAxnkxycjTpi31S\\nkM3gjazJXPY7rk5eVfDw6s8MqJgv3M03NTZtyuzlY81D5G3+rJ1K33ZroGhXt05K\\n1F3Qh6X6uayQYLlQzeVRXibbZTSFVEX5Bx0PAO2ZVe+yEItX3eobx5Gs/UW1/CW+\\nUwIDAQAB\\n-----END PUBLIC KEY-----\\n"
armenr commented 3 years ago

@arturocanalda

I think I know what the issue might be.

  1. It looks like you have an existing amplify-video category resource in your project which was created by the original plugin (not my version).
  2. It looks like you checked into stg from the existing dev backend env, at which time, the new version (my version) of the plugin went ahead and rendered a new set of props and resources for you.
  3. To solve your problem, I THINK the way to get through this is to check back into dev and run amplify video update --> and then choose to rotate your existing keys/create new keys.
  4. THEN, remove your stg props file, and check back into the stg backend env and see if all of the props render correctly (specifically, rPublicName).

If this works, it would confirm my suspicions and I might be able to figure out a fix for this (on the plugin side).

I'm guessing this might be the case because the code I added looks like this:

      const searchAndReplaceProps = () => {
        const newPropsObj = {};

        for (const [key, value] of Object.entries(existingPropsToMutate.contentDeliveryNetwork)) {
          // look for any string values that contain existing env's name
          if (typeof value === 'string' && value.includes(`${envNameToReplace}`)) {
            // replace with new env name
            const newValue = value.replace(new RegExp(envNameToReplace, 'g'), `${newEnvName}`);
            newPropsObj[key] = newValue;
          } else {
            // copy existing values that do not match replacement conditions aka "generic props"
            newPropsObj[key] = value;
          }
        }
        return newPropsObj;
      };

SO, the reason you're not seeing a rCloudFrontPublicKeystgvv37ttp5 (note the stg string in the name) is because the existing props file that the plugin is parsing and mutating values from (dev) contains the name rCloudFrontPublicKeyvv37ttp5 and NOT rCloudFrontPublicKeydevvv37ttp5.

Therefore, the easiest path forward would be to quickly check back into dev, go through the amplify update video CLI flow, let it re-gen your props and resources (by selecting to rotate your keys/create new keys). THEN, checking into stg should correctly generate the expected resource names.

I need to think more deeply about what forward-facing migration for existing users/existing resources looks like...so this is a good bug to uncover!

Please try what I've described above and let me know what the results are. Thanks again!

arturocanalda commented 3 years ago

It makes sense. I will do as you suggest on Monday first thing (this weekend is far too busy :)

arturocanalda commented 3 years ago

@armenr

I did as you suggested and it worked 👍
What also worked is amplify update video directly from my stg environment.

So, I can say the problem I was facing is fixed. Thank you for working on it so fast :)

armenr commented 3 years ago

@arturocanalda - Glad it's worked for you. It's been working really stable/well for me too!

@wizage - Anything I can do to help make sure this PR is compliant with expectations/merged? :)

nathanagez commented 3 years ago

@wizage did you had time to look at this PR ? Let me know if I can lighten your workload :)

wizage commented 3 years ago

LG2M - Will be expanding on this before releasing as some minor changes need to be made but overall needed.