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

support AWS creds as env vars (for deployment outside EKS) #235

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

Conversation

delocalizer
Copy link

@delocalizer delocalizer commented Jan 23, 2025

Context
I'm deploying Gen3 using helm but outside of EKS. As noted in the manifestservice documentation, 5daffd6 removed support for AWS credentials read from the /var/gen3/config/config.json file. As further suggested in that doc, a workaround for non-EKS users is to export the credentials to environment variables in the pod container. This PR is implementing the change for helm deployments foreshadowed in the doc here

Motivation
I want to deploy Gen3 outside of EKS context (e.g. for dev work on a local k8s cluster) and therefore can't use IRSA for auth. I want to use the environment variable workaround suggested in the documentation, with a simple helm deployment.

Overview
This PR uses the existing awsaccesskey and awssecretkey configuration points under manifestservice.manifestserviceG3auto to set optional environment variables AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY which will be used for credentials by calls without auth arguments to boto3.Session(), as for example in manifestservice.manifests.__init__.py.

If the key id and secret are not both present, the environment vars are not set.

Discussion
In the interests of a minimal change this PR does not remove AWS credentials written into config.json at MANIFEST_SERVICE_CONFIG_PATH, even though they do not appear to be used any more by manifestservice since 5daffd6 removed them from manifestservice.api.app.config. But perhaps they should be removed e.g. from here to reduce cruft? Is anyone relying on them being there?

Testing
I have tested this change in both the positive and noop cases (set the env vars, or don't, if the values.yaml values are not present), and it works for me.

New Features

  • Export AWS credentials to manifestservice env vars to support non-EKS deployment

Breaking Changes

Bug Fixes

Improvements

Dependency updates

Deployment changes

@delocalizer
Copy link
Author

Earlier PR Lint and Test Charts workflow run failed because the chart version hadn't been updated. This has been fixed.

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.

1 participant