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

Update supported SDKs for JS support #49

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

mikesir87
Copy link

The merge of aws/aws-sdk-js#2559 adds support to the JS AWS SDK.

Haven't tried it out yet, but will be doing so shortly. But, theoretically, it should work based on the change notes/pull request.

Signed-off-by: Michael Irwin mikesir87@gmail.com

The merge of aws/aws-sdk-js#2559 adds support to the JS AWS SDK.

Signed-off-by: Michael Irwin <mikesir87@gmail.com>
@mikkeloscar
Copy link
Contributor

I tried this before and it didn't work. However this prompted me to try again and I was able to make it work when setting ALL of these environment variables:

        - name: AWS_SDK_LOAD_CONFIG
          value: "true"
        # must be set for the AWS SDK/AWS CLI to find the credentials file.
        - name: AWS_SHARED_CREDENTIALS_FILE # used by js SDK
          value: /meta/aws-iam/credentials.process
        - name: AWS_CONFIG_FILE
          value: /meta/aws-iam/credentials.process
        - name: AWS_REGION
          value: eu-central-1

I have not check why AWS_SHARED_CREDENTIALS_FILE AND AWS_CONFIG_FILE must be set, but it doesn't work without from my testing.

We need a dedicated section for JS with this information similar to what we have for the other SDKs.

@mikesir87
Copy link
Author

Sounds great! I can add a dedicated JS section as well.

As a separate question... have you considered extending the controller to serve as a webhook to automatically add the secret mount and add the env vars? Would be awesome to make it mostly invisible to users. Curious on your thoughts...

@mikkeloscar
Copy link
Contributor

Sounds great! I can add a dedicated JS section as well.

Thanks!

As a separate question... have you considered extending the controller to serve as a webhook to automatically add the secret mount and add the env vars? Would be awesome to make it mostly invisible to users. Curious on your thoughts...

Definitely makes sense. We have not invested in this because we're looking more into adopting the solution from EKS at this point, but we would welcome contributions adding this.
One thing which I'm not 100% sure about is if some of the environment variables will conflict cross SDKs, then it's harder to just inject the settings in a webhook, because you need to know which SDK is in use.

@mikesir87
Copy link
Author

We have not invested in this because we're looking more into adopting the solution from EKS at this point, but we would welcome contributions adding this.

Yeah... we started with this route, but are running clusters not in EKS (either in AWS with a k8s platform or potentially even on-prem). And, trying to figure out how to get all of the pieces working has been tricky, which is why I looked for others and found this one. It's pretty straight forward and easy to understand what's going on.

One thing which I'm not 100% sure about is if some of the environment variables will conflict cross SDKs

Yeah... that was one of my worries too. Might be something that could be tested to figure out.

Anywho... I'll stop chatting about that here as it deserves its own issue/discussion thread. I'll update the MR for ya shortly 👍

Signed-off-by: Michael Irwin <mikesir87@gmail.com>
@mikesir87
Copy link
Author

Alrighty... I've added the snippet for the JS SDK. I didn't include a link to an example repo, as I couldn't find one. But, I did find a docker image for a js example and didn't see anything crazy in the app.js that would warrant reminders/additional setup.

@mikkeloscar
Copy link
Contributor

Signed-off-by: Michael Irwin <mikesir87@gmail.com>
Signed-off-by: Michael Irwin <mikesir87@gmail.com>
@mikesir87
Copy link
Author

And added the link to the repo. Thanks for making and pushing that 👍

@mikkeloscar
Copy link
Contributor

👍

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.

None yet

3 participants